Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/bin/
.openshift-install.log
/data/data/bin/
15 changes: 0 additions & 15 deletions cmd/openshift-install/version.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,8 @@ package main
import (
"fmt"
"os"
"os/exec"
"strings"

"github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"

"github.com/openshift/installer/pkg/terraform"
)

var (
Expand All @@ -28,14 +22,5 @@ func newVersionCmd() *cobra.Command {

func runVersionCmd(cmd *cobra.Command, args []string) error {
fmt.Printf("%s %s\n", os.Args[0], version)
terraformVersion, err := terraform.Version()
if err != nil {
exitError, ok := err.(*exec.ExitError)
if ok && len(exitError.Stderr) > 0 {
logrus.Error(strings.Trim(string(exitError.Stderr), "\n"))
}
return errors.Wrap(err, "Failed to calculate Terraform version")
}
fmt.Println(terraformVersion)
return nil
}
4 changes: 4 additions & 0 deletions data/unpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ func Unpack(base string, uri string) (err error) {
return nil
}

if err := os.MkdirAll(filepath.Dir(base), 0777); err != nil {
return err
}

out, err := os.OpenFile(base, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, 0666)
if err != nil {
return err
Expand Down
2 changes: 2 additions & 0 deletions hack/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ TAGS="${TAGS:-}"
OUTPUT="${OUTPUT:-bin/openshift-install}"
export CGO_ENABLED=0

(hack/get-terraform.sh)

case "${MODE}" in
release)
TAGS="${TAGS} release"
Expand Down
15 changes: 11 additions & 4 deletions hack/get-terraform.sh
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,15 @@ then
fi &&
TERRAFORM_VERSION="0.11.8" &&
TERRAFORM_URL="https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_${OS}_${ARCH}.zip" &&
echo "pulling ${TERRAFORM_URL}" >&2 &&
TERRAFORM_BINARY=".cache/terraform_${TERRAFORM_VERSION}_${OS}_${ARCH}"
cd "$(dirname "$0")/.." &&
mkdir -p bin &&
curl -L "${TERRAFORM_URL}" | "${FUNZIP}" >bin/terraform &&
chmod +x bin/terraform
if [ ! -x "${TERRAFORM_BINARY}" ]
then
echo "pulling ${TERRAFORM_URL}" >&2 &&
mkdir -p "$(dirname "${TERRAFORM_BINARY}")" &&
curl -L "${TERRAFORM_URL}" | "${FUNZIP}" >"${TERRAFORM_BINARY}" &&
chmod +x "${TERRAFORM_BINARY}"
fi &&
mkdir -p data/data/bin &&
rm -f data/data/bin/terraform &&
ln -s "../../../${TERRAFORM_BINARY}" data/data/bin/terraform
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wking do you know a good way of calculating "../../.." from "data/data/bin/terraform"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I came up with the following, but it may be more effort than it is worth:

diff --git a/hack/get-terraform.sh b/hack/get-terraform.sh
index aae1b69cd..c47aaa11e 100755
--- a/hack/get-terraform.sh
+++ b/hack/get-terraform.sh
@@ -25,6 +25,7 @@ fi &&
 TERRAFORM_VERSION="0.11.8" &&
 TERRAFORM_URL="https://releases.hashicorp.com/terraform/${TERRAFORM_VERSION}/terraform_${TERRAFORM_VERSION}_${OS}_${ARCH}.zip" &&
 TERRAFORM_BINARY=".cache/terraform_${TERRAFORM_VERSION}_${OS}_${ARCH}"
+TERRAFORM_SYMLINK="data/data/bin/terraform"
 cd "$(dirname "$0")/.." &&
 if [ ! -x "${TERRAFORM_BINARY}" ]
 then
@@ -33,6 +34,6 @@ then
        curl -L "${TERRAFORM_URL}" | "${FUNZIP}" >"${TERRAFORM_BINARY}" &&
        chmod +x "${TERRAFORM_BINARY}"
 fi &&
-mkdir -p data/data/bin &&
-rm -f data/data/bin/terraform &&
-ln -s "../../../${TERRAFORM_BINARY}" data/data/bin/terraform
+mkdir -p "$(dirname "${TERRAFORM_SYMLINK}")" &&
+rm -f "${TERRAFORM_SYMLINK}" &&
+ln -s "$(dirname "${TERRAFORM_SYMLINK}" | sed 's_[^/]*_.._g')/${TERRAFORM_BINARY}" "${TERRAFORM_SYMLINK}"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you know a good way of calculating "../../.." from "data/data/bin/terraform"?

Well, one way would be to use hardlinks instead of symlinks:

$ rm -f data/data/bin/terraform
$ ls -l .cache/terraform_0.11.8_linux_amd64 
-rwxr-xr-x. 1 trking trking 76422048 Dec  6 21:54 .cache/terraform_0.11.8_linux_amd64
$ ln .cache/terraform_0.11.8_linux_amd64 data/data/bin/terraform
$ ls -l .cache/terraform_0.11.8_linux_amd64 
-rwxr-xr-x. 2 trking trking 76422048 Dec  6 21:54 .cache/terraform_0.11.8_linux_amd64
$ ls -l data/data/bin/terraform 
-rwxr-xr-x. 2 trking trking 76422048 Dec  6 21:54 data/data/bin/terraform

See the <number of links> field changing from 1 to 2 after the ln call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume all of this will always be on the same filesystem, so a hardlink should work. I'll go with that instead.

5 changes: 2 additions & 3 deletions pkg/asset/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,10 @@ func (c *Cluster) Generate(parents asset.Parents) (err error) {
return fmt.Errorf("no known platform")
}

logrus.Infof("Using Terraform to create cluster...")
logrus.Infof("Creating cluster...")
stateFile, err := terraform.Apply(tmpDir, installConfig.Config.Platform.Name())
if err != nil {
err = errors.Wrap(err, "failed to run terraform")
err = errors.Wrap(err, "failed to create cluster")
}

data, err2 := ioutil.ReadFile(stateFile)
Expand All @@ -131,7 +131,6 @@ func (c *Cluster) Generate(parents asset.Parents) (err error) {
}
}

// TODO(yifan): Use the kubeconfig to verify the cluster is up.
return err
}

Expand Down
107 changes: 2 additions & 105 deletions pkg/terraform/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,59 +2,20 @@ package terraform

import (
"bytes"
"os"
"os/exec"
"path/filepath"
"runtime"
"strings"

"github.com/openshift/installer/pkg/lineprinter"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
)

// executor enables calling Terraform from Go, across platforms, with any
// additional providers/provisioners that the currently executing binary
// exposes.
//
// The Terraform binary is expected to be in the executing binary's folder, in
// the current working directory or in the PATH.
type executor struct {
binaryPath string
}

// Set the binary names for different platforms
const (
tfBinUnix = "terraform"
tfBinWindows = "terraform.exe"
)

// errBinaryNotFound denotes the fact that the Terraform binary could not be
// found on disk.
var errBinaryNotFound = errors.New(
"terraform not in executable's folder, cwd nor PATH",
)

// newExecutor initializes a new Executor.
func newExecutor() (*executor, error) {
ex := new(executor)

// Find the Terraform binary.
binPath, err := tfBinaryPath()
if err != nil {
return nil, errors.Wrap(err, "failed to get Terraform binary's path")
}

ex.binaryPath = binPath
return ex, nil
}

// Execute runs the given command and arguments against Terraform.
//
// An error is returned if the Terraform binary could not be found, or if the
// Terraform call itself failed, in which case, details can be found in the
// output.
func (ex *executor) execute(clusterDir string, args ...string) error {
func Execute(clusterDir string, args ...string) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to make this public. Can we change this (and the name in the godoc above) to execute?

// Prepare Terraform command by setting up the command, configuration,
// and the working directory
if clusterDir == "" {
Expand All @@ -67,7 +28,7 @@ func (ex *executor) execute(clusterDir string, args ...string) error {

stderr := &bytes.Buffer{}

cmd := exec.Command(ex.binaryPath, args...)
cmd := exec.Command(filepath.Join(clusterDir, executablePath), args...)
cmd.Dir = clusterDir
cmd.Stdout = linePrinter
cmd.Stderr = stderr
Expand All @@ -80,67 +41,3 @@ func (ex *executor) execute(clusterDir string, args ...string) error {
}
return err
}

// Version gets the output of 'terrraform version'.
func Version() (version string, err error) {
// Find the Terraform binary.
binPath, err := tfBinaryPath()
if err != nil {
return "", err
}

output, err := exec.Command(binPath, "version").Output()
if err != nil {
exitError := err.(*exec.ExitError)
if len(exitError.Stderr) == 0 {
exitError.Stderr = output
}
}
return strings.TrimRight(string(output), "\n"), err
}

// tfBinaryPath searches for a Terraform binary on disk:
// - in the executing binary's folder,
// - in the current working directory,
// - in the PATH.
// The first to be found is the one returned.
func tfBinaryPath() (string, error) {
// Depending on the platform, the expected binary name is different.
binaryFileName := tfBinUnix
if runtime.GOOS == "windows" {
binaryFileName = tfBinWindows
}

// Find the current executable's path, gets an absolute path or error
execPath, err := os.Executable()
if err == nil {
// execPath could be a symlink
if stat, err := os.Stat(execPath); err == nil && (stat.Mode()&os.ModeSymlink) == os.ModeSymlink {
if evalExecPath, err := filepath.EvalSymlinks(execPath); err != nil {
execPath = evalExecPath
}
}

// Look into the executable's folder.
path := filepath.Join(filepath.Dir(execPath), binaryFileName)
if stat, err := os.Stat(path); err == nil && !stat.IsDir() {
return path, nil
}
}

// Look into cwd.
if workingDirectory, err := os.Getwd(); err == nil {
path := filepath.Join(workingDirectory, binaryFileName)
if stat, err := os.Stat(path); err == nil && !stat.IsDir() {
return path, nil
}
}

// If we still haven't found the executable, look for it
// in the PATH.
if path, err := exec.LookPath(binaryFileName); err == nil {
return filepath.Abs(path)
}

return "", errBinaryNotFound
}
22 changes: 14 additions & 8 deletions pkg/terraform/terraform.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package terraform

import (
"fmt"
"os"
"path/filepath"

"github.com/openshift/installer/data"
Expand All @@ -10,17 +11,12 @@ import (

const (
// StateFileName is the default name for Terraform state files.
StateFileName string = "terraform.tfstate"
StateFileName string = "terraform.tfstate"
executablePath string = "bin/terraform"
)

func terraformExec(clusterDir string, args ...string) error {
// Create an executor
ex, err := newExecutor()
if err != nil {
return errors.Wrap(err, "failed to create Terraform executor")
}

err = ex.execute(clusterDir, args...)
err := Execute(clusterDir, args...)
if err != nil {
return errors.Wrap(err, "failed to execute Terraform")
}
Expand Down Expand Up @@ -93,6 +89,16 @@ func unpackAndInit(dir string, platform string) (err error) {
return errors.Wrap(err, "failed to unpack Terraform modules")
}

err = data.Unpack(filepath.Join(dir, executablePath), filepath.FromSlash(executablePath))
if err != nil {
return errors.Wrap(err, "failed to unpack Terraform binary")
}

err = os.Chmod(filepath.Join(dir, executablePath), 0555)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need FromSlash too, and so will exec.Command(filepath.Join(clusterDir, executablePath), args...). In fact, why are we bothering with bin at all? This would be easier if we just had executableFilename = "terraform", or some such. And since we have CI to catch us if we typo an instance, I'd be fine hard-coding "terraform"everywhere and skipping the variable altogether (although I'm also fine with a slash-lessexecutableFilename`).

if err != nil {
return errors.Wrap(err, "failed to make Terraform binary executable")
}

err = terraformExec(dir, "init", "-input=false", "-no-color")
if err != nil {
return errors.Wrap(err, "failed to initialize Terraform")
Expand Down