Previously, destroy support was behind TAGS=libvirt_destroy and create
support was always built in. But since 3fb4400c (terraform/plugins:
add `libvirt`, `aws`, `ignition`, `openstack` to KnownPlugins,
2018-12-14, #919), the bundled libvirt Terraform provider has also
been behind libvirt_destroy. That leads to cluster creation failing
with:
$ openshift-install create cluster
...
ERROR Missing required providers.
ERROR
ERROR The following provider constraints are not met by the currently-installed
ERROR provider plugins:
ERROR
ERROR * libvirt (any version)
ERROR
ERROR Terraform can automatically download and install plugins to meet the given
ERROR constraints, but this step was skipped due to the use of -get-plugins=false
ERROR and/or -plugin-dir on the command line.
...
With this commit, folks trying to 'create cluster' without libvirt
compiled in will get:
FATAL failed to fetch Common Manifests: failed to load asset "Install Config": invalid "install-config.yaml" file: platform: Invalid value: types.Platform{AWS:(*aws.Platform)(nil), Libvirt:(*libvirt.Platform)(0xc4209511f0), OpenStack:(*openstack.Platform)(nil)}: platform must be one of: aws, openstack
before we get to Terraform.
Now that the build tag guards both creation and deletion, I've renamed
it from 'libvirt_destroy' to the unqualified 'libvirt'.
I've also adjusted the install-config validation testing to use
regular expressions so we can distinguish between failures because
libvirt was not compiled in as a valid platform and failures because
some portion of the libvirt configuration was broken. In order to get
stable error messages for comparison, I've added some strings.Sort
calls for various allowed-value string-slice computations.
One environment variable to be used when building the pinned release binary:
RHCOS_BUILD_NAME: a string representing the build name(or rather number e.g. 47.48). If empty, then the latest one will be picked up.
This could just be hardcoded in the hack/build.sh script before making the release branch so that it stays baked in.
But still leave an override env var so that it can be overridden. Use the following env var to pin the image while building the binary:
```
$ # export the release-image variable
$ export RELEASE_IMAGE=registry.redhat.io/openshift/origin-release:v4.0"
$ # build the openshift-install binary
$ ./hack/build.sh
$ # distribute the binary to customers and run with pinned image being picked up
$ ./bin/openshift-install create cluster...
```
The only way to override it would be to use an env var OPENSHIFT_INSTALL_RELEASE_IMAGE_OVERRIDE while running the openshift-install binary.
Now that we're vendoring Terraform, we no longer need a script to pull
down pre-compiled binaries.
I've also reworded the 'create cluster' description in overview.md to
be more generic. See 0c3d8658 (asset/cluster: clean up logging
messages, 2018-12-07, #840) for similar changes to another location.
This gets the correct main.version value. Before this commit, you
could have:
$ hack/release.sh v0.4.0
...
+ go build -ldflags ' -X main.version=v0.3.0-273-g...' ...
...
With this commit, that same invocation will give you:
+ go build -ldflags ' -X main.version=v0.4.0' ...
The bug is from the initial script in 7b023303 (hack/release: Make it
easy to cross-compile release binaries, 2018-10-02, #397).
This makes the config a bit easier to read, and also supports folks
who want to run yamllint directly (without going through
hack/yaml-lint.sh).
I've added the document-start and indentation rules to avoid
complaints like:
$ hack/yaml-lint.sh
...
./data/data/bootstrap/files/opt/tectonic/manifest-overrides/kube-proxy-daemonset.yaml
3:1 warning missing document start "---" (document-start)
23:7 error wrong indentation: expected 8 but found 6 (indentation)
24:9 error wrong indentation: expected 10 but found 8 (indentation)
31:9 error wrong indentation: expected 10 but found 8 (indentation)
40:9 error wrong indentation: expected 10 but found 8 (indentation)
49:7 error wrong indentation: expected 8 but found 6 (indentation)
51:7 error wrong indentation: expected 8 but found 6 (indentation)
...
This reverts commit 8ff1cee1ce
(2018-09-28, #370).
We moved from Glide to dep in 1f455431 (vendor: switch from glide to
dep, 2018-09-28, #380), so we no longer need to worry about yamllint
vs. Glide.yaml.
This seems to be a very common mistake when people try building the
installer. Do a sanity check to catch this and make the error more
clear.
Co-authored-by: W. Trevor King <wking@tremily.us>
And stick Git tagging in there too, because why not. This is mostly
to get our list of target OSes and architectures into version control.
The SKIP_GENERATION variable allows you to turn off vfsgen when we
have GOOS and/or GOARCH set to values that will not run on your host
;).
Also change --abbrev from 0 to 40 in the build script, because
--abbrev=0 results in dropping the commit hash entirely when there's a
past tag:
$ git describe --always --abbrev=40 --dirty
v0.1.0-2-gff5ee75e5010efb305d4bc381d944b14a4a97c3b-dirty
$ git describe --always --abbrev=0 --dirty
v0.1.0-dirty
$ git describe --always --dirty
v0.1.0-2-gff5ee75-dirty
Checking on the tag itself for sanity:
$ git checkout v0.1.0
$ git describe --always --abbrev=40 --dirty
v0.1.0
I'd added this in f230c3e9 (hack/build: Add the local vendor directory
for generate, 2018-09-26, #340), citing [1]. But:
* vendor/ is not a useful GOPATH addition, because it lacks the src/
that Go requires. Go might look in vendor/src/github.com/... with
our GOPATH stuffing, but that won't help it find our vendored
libraries.
* All that really seems to matter is whether the repository as a whole
is checked out into your GOPATH.
I'm really looking forward to Go 1.11's modules killing the GOPATH
idea.
[1]: https://github.com/golang/go/issues/20406
Reported-by: Stephen Augustus <saugustu@redhat.com>
And calculate its value based on the most recent Git tag (relative to
HEAD). This makes it easy for callers to report the version they're
using when reporting issues (or successes :).
Docs for Go's build constraints are in [1]. This commit allows folks
with local libvirt C libraries to compile our libvirt deletion logic
(and get a dynamically-linked executable), while release binaries and
folks without libvirt C libraries can continue to get
statically-linked executables that lack libvirt deletion.
I've also simplified the public names (e.g. NewDestroyer -> New),
dropping information which is already encoded in the import path.
Pulling the init() registration out into separate files is at
Abhinav's request [2].
[1]: https://golang.org/pkg/go/build/#hdr-Build_Constraints
[2]: https://github.com/openshift/installer/pull/387#discussion_r221763315
Temporarily make the script a no-op while we work out whether we want
to keep this. It's currently complaining about the YAML output from
Glide in the previous commit [1]:
./glide.yaml
1:1 warning missing document start "---" (document-start)
3:1 error wrong indentation: expected 2 but found 0 (indentation)
5:1 error wrong indentation: expected 2 but found 0 (indentation)
38:1 error wrong indentation: expected 2 but found 0 (indentation)
Dropping the script entirely would break CI [2]. Alex is ok dropping
yamllint entirely, but I'm half-interested in keeping it around in
case we decide to move any static YAML assets
(e.g. pkg/asset/manifests/content/tectonic/rbac/role-user.go) over
into data/data.
[1]: https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_installer/370/pull-ci-openshift-installer-yaml-lint/636/build-log.txt
[2]: d7ea5b36da/ci-operator/jobs/openshift/installer/openshift-installer-master-presubmits.yaml (L354)
Michael had gunzip installed, but not funzip [1], and while funzip is
specifically written to extract the first file from a ZIP archive,
gunzip seems to be able to do the same thing (although I haven't found
docs to back this up).
This commit addss a helper function 'have' to get a more compact
'command' call. And it adds a new FUNZIP variable (which the user can
override if they want, using the ${parameter:-word} syntax [2])
defaulting to 'funzip'. If the initial command is missing, we fall
back to gunzip if it's available, and finally exit with an error if it
wasn't.
There's a race between the initial "do we have ${FUNZIP}?" check and
the final 'command -V' [3] call used to generate the "not found" error
message. But I don't expect many devs to supply the ${FUNZIP} command
during the brief window between checks.
[1]: https://github.com/openshift/installer/pull/356#discussion_r221354378
[2]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02
[3]: http://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html
Rather than just checking the old directories, we'd might as well test
the entire repository. This also updates all of the yaml files, making
them more readable, in my opinion.
Jeff Ligon is on OS X. With this change, we use Go to give the
correct OS and architecture constants for the Terraform URL. Go,
because Terraform is a Go project, so Hashicorp is building the
binaries using Go's cross-compilation with the Go naming.
If you don't have Go, I'm just assuming you're on amd64 Linux, because
that's what our CI infrastructure is running.
Default to 'release' to make life easier for openshift/release, and
because a release build will work more reliably (the Terraform assets
are always there, they're just harder to change).
This is mostly for CI, so we can give the release repo a stable script
to run. And we can bump our Terraform version in the script here
without bothering with release pull-requests.
We want to stuff these in when the user gave us no arguments, not
clobber arguments when the user passed some in :p. Fixes a bug from
4b9cbdd9 (hack/tf-fmt: Allow callers to pass positional args,
2018-09-13, #248).
Setting --workdir and passing ${@} through means:
$ ./hack/shellcheck.sh
will show any errors with relative paths that will still work outside
the container. For example:
./tests/smoke/.build/external/go_sdk/src/syscall/mkall.sh:278:7: note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]
instead of:
/workdir/tests/smoke/.build/external/go_sdk/src/syscall/mkall.sh:278:7: note: Expressions don't expand in single quotes, use double quotes for that. [SC2016]
Adding prune rules for vendor and .build under tests/smoke avoids
complaining about that file and others over which are maintained
upstream or elsewhere.
Eric points out potential issues with relabeling /tmp [1], which is
shared by several system-level consumers.
For the Bazel script, the /tmp mount is just since c483f597 (Move
bazel build tarball test to prow, 2018-08-08, #117), so we can drop it
to return to our previous approach.
The Terraform container seems to run fine without /tmp as well,
although there's no clear history to point to on this front because we
used to use Bazel for this. See b8a9bbc5 (Remove bazel from test
process, 2018-08-01, #97).
[1]: https://github.com/openshift/installer/pull/174#discussion_r212764528
Like we did in bootkube.sh in 0fa4eb13 (Fix perm errors with selinux
enabled, 2018-08-15, #134). This gives us permission to access the
mounted volume when SELinux is enabled (docs in [1]).
I've also normalized these invocations for consistency between the
various hack/ scripts:
* Adding slash separators to put each option on its own line,
excepting the final command being run in the container. This makes
the long commands slightly easier to skim. It will also make it
easier to track down motivation for an option with 'git blame',
because commits touching options on other lines won't clutter the
blame.
* Use long-form options (-v -> --volume, etc.). This makes the
options a bit more accessible to newcomers, and now that each option
is on it's own line we have plenty of space.
* Dropped single quotes from 'TRUE'. There are no shell-sensitive
characters in TRUE, so there's no need to quote it.
* Use ${PWD} consistently. It's in POSIX [2], so there's no need to
execute a pwd process to get this value.
* Drop -t. None of these commands should need a pseudoterminal.
* Drop explicit rw --volume options. They're the default [3].
[1]: https://github.com/containers/libpod/blame/v0.8.3/docs/podman-run.1.md#L628
[2]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html#tag_08_03
[3]: https://github.com/containers/libpod/blame/v0.8.3/docs/podman-run.1.md#L646
Catching up with 4e92db8 (Merge pull request #120 from
staebler/asset_graph_engine, 2018-08-24).
* Drop the go-lint comment from go-fmt.sh. I'd accidentally
copy/pasted this over in 87b3e170 (hack/go-fmt: Make it easy to
auto-format Go, 2018-08-24, #173).
* Suggest users run go-lint over pkg/... as well.
* Simplify the 'go vet' invocation. The code I'm removing is from
eb71c8de (Added go-vet shell script, 2018-08-06, #110). The Travis
config it replaced was removed in 1765e93c (.travis.yml: Prune
moved-to-Prow tf-lint, etc., 2018-08-14, #123), but was just
running:
go vet ./installer/...
inside the container. So I don't think we need the directory
changing or moving here. And we should certainly be able to cover
the test from a single container, instead of launch a new container
for each requested directory (at least as long as the requested
directories are under $PWD, and the old script required that
anyway).
So users and CI tooling can keep our Go clean :).
The find call avoids formatting .build/* (Bazel output) and vendor/*
(controlled upstream), because gofmt has no special vendor/ handling
built in [1].
Formatting our Go and then using 'git diff' to show the changes (and
error if there were any) makes it easy for:
* CI to see if there were issues (because of the exit code).
* Users to see the required changes in the CI logs (because of the
output diff).
[1]: https://github.com/golang/go/issues/22173#issuecomment-338782431
This allows callers to set build options [1] if needed. The Prow
tests need this to set HOME to avoid [2]:
ERROR: /home/prow/go/src/github.com/openshift/installer/BUILD.bazel:106:1: error executing shell command: 'bazel-out/host/bin/external/bazel_tools/tools/build_defs/pkg/build_tar --flagfile=bazel-out/k8-fastbuild/bin/tf_bin.args' failed (Exit 1)
Traceback (most recent call last):
File "/usr/lib/python2.7/site.py", line 554, in <module>
main()
File "/usr/lib/python2.7/site.py", line 536, in main
known_paths = addusersitepackages(known_paths)
File "/usr/lib/python2.7/site.py", line 272, in addusersitepackages
user_site = getusersitepackages()
File "/usr/lib/python2.7/site.py", line 247, in getusersitepackages
user_base = getuserbase() # this will also set USER_BASE
File "/usr/lib/python2.7/site.py", line 237, in getuserbase
USER_BASE = get_config_var('userbase')
File "/usr/lib/python2.7/sysconfig.py", line 582, in get_config_var
return get_config_vars().get(name)
File "/usr/lib/python2.7/sysconfig.py", line 533, in get_config_vars
_CONFIG_VARS['userbase'] = _getuserbase()
File "/usr/lib/python2.7/sysconfig.py", line 210, in _getuserbase
return env_base if env_base else joinuser("~", ".local")
File "/usr/lib/python2.7/sysconfig.py", line 196, in joinuser
return os.path.expanduser(os.path.join(*args))
File "/usr/lib/python2.7/posixpath.py", line 262, in expanduser
userhome = pwd.getpwuid(os.getuid()).pw_dir
KeyError: 'getpwuid(): uid not found: 1000130000'
The chain for that crash is:
1. Bazel invokes build_tar via run_shell with
use_default_shell_env=True [3], so only environment variables
declared with --action_env [1] are exposed to build_tar.
2. Python sees $HOME is empty and falls back to
pwd.getpwuid(os.getuid()).pw_dir [4].
3. OpenShift Prow containers execute with arbitrary container-side
UIDs [5], so the getpwuid call fails.
With this commit, Prow can setup --action_env to work around its
arbitrary container-side UIDs [6].
[1]: https://docs.bazel.build/versions/master/command-line-reference.html#build-options
[2]: https://storage.googleapis.com/origin-ci-test/pr-logs/pull/openshift_installer/123/ci-pull-openshift-installer-bazel-build-tarball/4/build-log.txt
[3]: https://github.com/bazelbuild/bazel/blob/0.16.1/tools/build_defs/pkg/pkg.bzl#L82-L89
[4]: 1f34aece28/Lib/posixpath.py (L260-L262)
[5]: https://github.com/openshift/release/pull/1178#issuecomment-412965330
[6]: https://github.com/openshift/release/pull/1185
