Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Aug 1, 2018

In #92 I'm having some difficulty updating our vendored packages to drop go-jose and jwt-go. This pull request updates our smoke-test vendoring. It also updates some stale imports in our local tests, but makes no other local changes. If this passes the tests, we can merge it and then I can rebase #92 on top to make it easier to focus on the semantic change #92 is making. There are more details on the individual changes in the commit messages.

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 1, 2018
@wking wking added run-smoke-tests and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Aug 1, 2018
@wking wking force-pushed the smoke-test-vendor branch from 91041f1 to 726ebd8 Compare August 1, 2018 20:42
@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Aug 1, 2018
@wking wking force-pushed the smoke-test-vendor branch from 726ebd8 to 837363c Compare August 1, 2018 21:36
@wking
Copy link
Member Author

wking commented Aug 1, 2018

The Jenkins error was:

21:40:25 [378 / 1,001] GoCompile vendor/k8s.io/apimachinery/pkg/apis/meta/v1/linux_amd64_pure_stripped/go_default_library~/installer/vendor/k8s.io/apimachinery/pkg/apis/meta/v1.a; 1s processwrapper-sandbox ... (4 actions running)
21:40:32 ERROR: /home/ubuntu/workspace/openshift-installer_PR-94-FV2HNHYJPVBF5GZ3MH26OZVF4QMKHQIHHHOZ7G4F4LD7QHW3DUPA/tests/smoke/vendor/k8s.io/apimachinery/pkg/runtime/serializer/json/BUILD.bazel:3:1: GoCompile tests/smoke/vendor/k8s.io/apimachinery/pkg/runtime/serializer/json/linux_amd64_stripped/go_default_library~/installer/tests/smoke/vendor/k8s.io/apimachinery/pkg/runtime/serializer/json.a failed (Exit 1)
21:40:32 /home/ubuntu/workspace/openshift-installer_PR-94-FV2HNHYJPVBF5GZ3MH26OZVF4QMKHQIHHHOZ7G4F4LD7QHW3DUPA/.build/sandbox/processwrapper-sandbox/483/execroot/installer/tests/smoke/vendor/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go:104:16: unknown field 'CaseSensitive' in struct literal of type jsoniter.Config
21:40:32 GoCompile: error running subcommand: exit status 1

which sounds like kubernetes/apimachinery#46. I've pushed d4e871c8dfef049a0923ed45bf3eb772c3749c63 to work around it.

@wking wking changed the title tests/smoke/vendor: Bump our deps and use testImport wip: tests/smoke/vendor: Bump our deps and use testImport Aug 2, 2018
@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 2, 2018
@wking wking force-pushed the smoke-test-vendor branch from d4e871c to 8762262 Compare August 2, 2018 20:04
wking added 8 commits August 3, 2018 08:55
Generated with:

  $ bazel run //:gazelle

Using:

  $ bazel version
  Build label: 0.15.2- (@Non-Git)
  Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
  Build time: Tue Jul 17 13:32:28 2018 (1531834348)
  Build timestamp: 1531834348
  Build timestamp as int: 1531834348
The 'smoke' target is a lot like the Gazelle-maintained
go_default_test target, except that it's not automatically maintained.
Make maintenance easier by pointing the smoke_tests alias straight at
the automatically-maintained target.  Generated with:

  $ rm smoke/tests/BUILD.bazel
  $ bazel run //:gazelle
  $ emacs smoke/tests/BUILD.bazel  # add visibility

Adding the visibility property avoids:

  $ bazel run smoke_tests
  ERROR: /home/trking/.local/lib/go/src/github.com/openshift/installer/BUILD.bazel:45:1: target '//tests/smoke:go_default_test' is not visible from target '//:smoke_tests'. Check the visibility declaration of the former target if you think the dependency is legitimate
  ERROR: Analysis of target '//:smoke_tests' failed; build aborted: Analysis of target '//:smoke_tests' failed; build aborted
  INFO: Elapsed time: 0.124s
  INFO: 0 processes.
  FAILED: Build did NOT complete successfully (1 packages loaded)
  FAILED: Build did NOT complete successfully (1 packages loaded)

Running the smoke tests is also fairly orthogonal to building
tarballs, so I've removed the smoke-test docs from
Documentation/dev/build.md.

The last test_vars consumer was removed in 7296010 (frontend: Remove
frontend code and backend API code, 2018-03-06,
coreos/tectonic-installer#3067), so I don't think its removal will be
a problem.  And because smoke.sh was removed in 1dea5c8 (tests:
Remove unused smoke.sh + tfvars file, 2017-10-04,
coreos/tectonic-installer#2036), I've just removed the whole
tests/smoke/aws tree.  And without that tree to explain, I've dropped
the associated section from the smoke README as well.
This change is similar to kubernetes/kubernetes@f2d3220a (run
root-rewrite-import-client-go-api-types, 2017-06-22,
kubernetes/kubernetes#44784, v1.8.0-alpha.2) and it protects us from
the eventual removal of k8s.io/client-go/pkg/api/v1 in
kubernetes/kubernetes@ffe74d1f (run hack/update-staging-client-go,
2017-06-22, kubernetes/kubernetes#44784, v1.8.0-alpha.2).
This change is similar to kubernetes/kubernetes@fd044d15 (fix dynamic
client name, 2018-05-09, kubernetes/kubernetes#63446, v1.11.0-beta.0)
and it protects us from the eventual removal of
k8s.io/kubernetes/pkg/kubectl/resource in
kubernetes/kubernetes@16d6a6c5 (move resource builder to generic
options, 2018-05-10, kubernetes/kubernetes#63669, v1.11.0-beta.0).
Start a fresh Glide setup to use testImport [1]:

  $ cd tests/smoke
  $ rm -rf glide.yaml glide.lock vendor
  $ glide init
  ...
  [INFO]Writing configuration file (glide.yaml)
  [INFO]Would you like Glide to help you find ways to improve your glide.yaml configuration?
  [INFO]If you want to revisit this step you can use the config-wizard command at any time.
  [INFO]Yes (Y) or No (N)?
  y
  ...
  [INFO]The package k8s.io/client-go appears to have Semantic Version releases (http://semver.org).
  [INFO]The latest release is v8.0.0. You are currently not using a release. Would you like
  [INFO]to use this release? Yes (Y) or No (N)
  y
  [INFO]Would you like to remember the previous decision and apply it to future
  [INFO]dependencies? Yes (Y) or No (N)
  y
  ...
  [INFO]The package k8s.io/client-go appears to use semantic versions (http://semver.org).
  [INFO]Would you like to track the latest minor or patch releases (major.minor.patch)?
  [INFO]The choices are:
  [INFO] - Tracking minor version releases would use '>= 8.0.0, < 9.0.0' ('^8.0.0')
  [INFO] - Tracking patch version releases would use '>= 8.0.0, < 8.1.0' ('~8.0.0')
  [INFO] - Skip using ranges
  [INFO]For more information on Glide versions and ranges see https://glide.sh/docs/versions
  [INFO]Minor (M), Patch (P), or Skip Ranges (S)?
  m
  [INFO]Would you like to remember the previous decision and apply it to future
  [INFO]dependencies? Yes (Y) or No (N)
  y
  [INFO]Configuration changes have been made. Would you like to write these
  [INFO]changes to your configuration file? Yes (Y) or No (N)
  y
  [INFO]Writing updates to configuration file (glide.yaml)
  ...

client-go 8.0 is aligned with Kubernetes 1.11 [2], so roll back
Kubernetes from 1.12 to 1.11 and only allow patch releases [3]:

  $ sed -i 's/\^1\.12\.0-alpha\.0/~1.11.0/' glide.yaml

Then:

  $ glide update
  ...
  [WARN]Conflict: k8s.io/client-go version is ^8.0.0, but also asked for d6a5799477e64ff9e476894020e68cfe794bbeb5
  [INFO]k8s.io/client-go reference d6a5799477e64ff9e476894020e68cfe794bbeb5:
  [INFO] - author: Kubernetes Publisher <[email protected]>
  [INFO] - commit date: Thu, 19 Jul 2018 09:29:00 -0700
  [INFO] - subject (first line): Merge pull request #65771 from smarterclayton/untyped
  [INFO]Keeping k8s.io/client-go ^8.0.0
  ...

I'm not going to worry about that at the moment, since the rest
of the command went smoothly.  Then:

  $ glide install --strip-vendor
  $ glide-vc --use-lock-file --no-tests --only-code
  $ bazel run //:gazelle

using:

  $ (cd $GOPATH/src/github.com/Masterminds/glide && git describe)
  v0.13.1-7-g3e13fd1
  $ (cd $GOPATH/src/github.com/sgotti/glide-vc && git describe)
  v0.1.0-2-g6ddf6ee
  $ bazel version
  WARNING: --batch mode is deprecated. Please instead explicitly shut down your Bazel server using the command "bazel shutdown".
  Build label: 0.15.2- (@Non-Git)
  Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
  Build time: Tue Jul 17 13:32:28 2018 (1531834348)
  Build timestamp: 1531834348
  Build timestamp as int: 1531834348

This vendors a few new packages, like k8s.io/api/apps/v1.  We could
remove those by dropping --use-lock-file, but unfortunately that
causes glide-vc to remove *all* of our vendored packages.  And Glide
is stuffing all of those subpackages into glide.lock.  There doesn't
seem to be a way to get glide-vc to respect testImport (or
alternatively to scan dependencies for the local tests itself) but
still remove any *vendored* tests.
Work around [1], which was causing smoke-test errors like [2]:

  21:40:25 [378 / 1,001] GoCompile vendor/k8s.io/apimachinery/pkg/apis/meta/v1/linux_amd64_pure_stripped/go_default_library~/installer/vendor/k8s.io/apimachinery/pkg/apis/meta/v1.a; 1s processwrapper-sandbox ... (4 actions running)
  21:40:32 ERROR: /home/ubuntu/workspace/openshift-installer_PR-94-FV2HNHYJPVBF5GZ3MH26OZVF4QMKHQIHHHOZ7G4F4LD7QHW3DUPA/tests/smoke/vendor/k8s.io/apimachinery/pkg/runtime/serializer/json/BUILD.bazel:3:1: GoCompile tests/smoke/vendor/k8s.io/apimachinery/pkg/runtime/serializer/json/linux_amd64_stripped/go_default_library~/installer/tests/smoke/vendor/k8s.io/apimachinery/pkg/runtime/serializer/json.a failed (Exit 1)
  21:40:32 /home/ubuntu/workspace/openshift-installer_PR-94-FV2HNHYJPVBF5GZ3MH26OZVF4QMKHQIHHHOZ7G4F4LD7QHW3DUPA/.build/sandbox/processwrapper-sandbox/483/execroot/installer/tests/smoke/vendor/k8s.io/apimachinery/pkg/runtime/serializer/json/json.go:104:16: unknown field 'CaseSensitive' in struct literal of type jsoniter.Config
  21:40:32 GoCompile: error running subcommand: exit status 1

Generated with:

  $ cd tests/smoke
  $ glide get --test github.com/json-iterator/go#f2b4162afba35581b6d4a50d3b8f34e33c144682
  ...
  [WARN]Conflict: github.com/json-iterator/go rev is currently f2b4162afba35581b6d4a50d3b8f34e33c144682, but k8s.io/api wants 2ddf6d758266fcb080a4f9e054b9f292c85e6798
  [INFO]github.com/json-iterator/go reference f2b4162afba35581b6d4a50d3b8f34e33c144682:
  [INFO] - author: Tim Hockin <[email protected]>
  [INFO] - commit date: Tue, 12 Jun 2018 13:28:35 -0700
  [INFO] - subject (first line): Merge pull request openshift#285 from nikhita/fix-case-sensitivity
  [INFO]github.com/json-iterator/go reference 2ddf6d758266fcb080a4f9e054b9f292c85e6798:
  [INFO] - author: Tao Wen <[email protected]>
  [INFO] - commit date: Tue, 24 Apr 2018 08:46:23 +0800
  [INFO] - subject (first line): Merge pull request openshift#266 from ceshihao/fix_base64_with_whitespace
  [INFO]Keeping github.com/json-iterator/go f2b4162afba35581b6d4a50d3b8f34e33c144682
  ...
  [WARN]Conflict: k8s.io/client-go version is ^8.0.0, but also asked for d6a5799477e64ff9e476894020e68cfe794bbeb5
  [INFO]k8s.io/client-go reference d6a5799477e64ff9e476894020e68cfe794bbeb5:
  [INFO] - author: Kubernetes Publisher <[email protected]>
  [INFO] - commit date: Thu, 19 Jul 2018 09:29:00 -0700
  [INFO] - subject (first line): Merge pull request #65771 from smarterclayton/untyped
  [INFO]Keeping k8s.io/client-go ^8.0.0
  ...
  $ glide install --strip-vendor
  $ glide-vc --use-lock-file --no-tests --only-code
  $ bazel run //:gazelle

using:

  $ (cd $GOPATH/src/github.com/Masterminds/glide && git describe)
  v0.13.1-7-g3e13fd1
  $ (cd $GOPATH/src/github.com/sgotti/glide-vc && git describe)
  v0.1.0-2-g6ddf6ee
  $ bazel version
  WARNING: --batch mode is deprecated. Please instead explicitly shut down your Bazel server using the command "bazel shutdown".
  Build label: 0.15.2- (@Non-Git)
  Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
  Build time: Tue Jul 17 13:32:28 2018 (1531834348)
  Build timestamp: 1531834348
  Build timestamp as int: 1531834348

The client-go conflict reported above is not new to this commit; see
837363cf (tests/smoke/vendor: Rebuild with testImport, 2018-08-01, openshift#94).

[1]: operator-framework/getting-started#16
[2]: https://jenkins-tectonic-installer.prod.coreos.systems/job/openshift-installer/view/change-requests/job/PR-94/lastBuild/console
And use the (hopefully more stable) client-go and apimachinery
analogs.  Also transition to kubernetes.Interface, which is more
generic than *Clientset.  These changes avoid:

  $ go test ./tests/smoke/
  # github.com/openshift/installer/tests/smoke
  tests/smoke/cluster_test.go:319:25: cannot use cfg (type clientcmd.ClientConfig) as type genericclioptions.RESTClientGetter in argument to util.NewFactory:
      clientcmd.ClientConfig does not implement genericclioptions.RESTClientGetter (missing ToDiscoveryClient method)
  tests/smoke/cluster_test.go:321:28: too many arguments in call to f.Validator
      have (bool, string)
      want (bool)
  tests/smoke/cluster_test.go:326:25: f.UnstructuredObject undefined (type util.Factory has no field or method UnstructuredObject)
  tests/smoke/cluster_test.go:331:31: too many arguments in call to "github.com/openshift/installer/tests/smoke/vendor/k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource".NewBuilder
  tests/smoke/cluster_test.go:331:41: f.CategoryExpander undefined (type util.Factory has no field or method CategoryExpander)
  tests/smoke/cluster_test.go:331:69: undefined: "github.com/openshift/installer/tests/smoke/vendor/k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource".ClientMapperFunc
  tests/smoke/cluster_test.go:401:20: info.VersionedObject undefined (type *"github.com/openshift/installer/tests/smoke/vendor/k8s.io/kubernetes/pkg/kubectl/genericclioptions/resource".Info has no field or method VersionedObject)
  FAIL    github.com/openshift/installer/tests/smoke [build failed]

due to our recent vendor version bumps.

Also move from Core() to CoreV1() to catch up with
kubernetes/kubernetes@c679fee1 (Update staging client-go, 2016-11-08,
kubernetes/kubernetes#36465) which deprecated the unversioned Core().
Generated with:

  $ cd tests/smoke
  $ rm -rf BUILD.bazel glide.yaml glide.lock vendor
  $ glide init
  ...
  [INFO]Would you like Glide to help you find ways to improve your glide.yaml configuration?
  [INFO]If you want to revisit this step you can use the config-wizard command at any time.
  [INFO]Yes (Y) or No (N)?
  y
  ...
  [INFO]The package k8s.io/client-go appears to have Semantic Version releases (http://semver.org).
  [INFO]The latest release is v8.0.0. You are currently not using a release. Would you like
  [INFO]to use this release? Yes (Y) or No (N)
  y
  [INFO]Would you like to remember the previous decision and apply it to future
  [INFO]dependencies? Yes (Y) or No (N)
  y
  [INFO]Updating k8s.io/client-go to use the release v8.0.0 instead of no release
  [INFO]The package k8s.io/client-go appears to use semantic versions (http://semver.org).
  [INFO]Would you like to track the latest minor or patch releases (major.minor.patch)?
  [INFO]The choices are:
  [INFO] - Tracking minor version releases would use '>= 8.0.0, < 9.0.0' ('^8.0.0')
  [INFO] - Tracking patch version releases would use '>= 8.0.0, < 8.1.0' ('~8.0.0')
  [INFO] - Skip using ranges
  [INFO]For more information on Glide versions and ranges see https://glide.sh/docs/versions
  [INFO]Minor (M), Patch (P), or Skip Ranges (S)?
  m
  [INFO]Would you like to remember the previous decision and apply it to future
  [INFO]dependencies? Yes (Y) or No (N)
  y
  [INFO]Updating k8s.io/client-go to use the range ^8.0.0 instead of commit id v8.0.0
  [INFO]Configuration changes have been made. Would you like to write these
  [INFO]changes to your configuration file? Yes (Y) or No (N)
  y
  [INFO]Writing updates to configuration file (glide.yaml)
  ...
  $ glide get --test github.com/json-iterator/go#f2b4162afba35581b6d4a50d3b8f34e33c144682
  $ glide update --strip-vendor
  $ glide-vc --use-lock-file --no-tests --only-code
  $ bazel run //:gazelle
  $ emacs BUILD.bazel  # restore the visibility property

using:

  $ (cd $GOPATH/src/github.com/Masterminds/glide && git describe)
  v0.13.1-7-g3e13fd1
  $ (cd $GOPATH/src/github.com/sgotti/glide-vc && git describe)
  v0.1.0-2-g6ddf6ee
  $ bazel version
  Build label: 0.16.0- (@Non-Git)
  Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
  Build time: Tue Jul 31 18:27:17 2018 (1533061637)
  Build timestamp: 1533061637
  Build timestamp as int: 1533061637

The json-iterator pinning is restoring the change from 648128bd
(tests/smoke/vendor: Pin json-iterator at f2b4162a, 2018-08-01).
@wking wking force-pushed the smoke-test-vendor branch from 8762262 to 6642cff Compare August 3, 2018 16:10
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 3, 2018
@wking
Copy link
Member Author

wking commented Aug 3, 2018

The Jenkins error was:

1 error(s) occurred:

* module.masters.aws_autoscaling_group.masters: 1 error(s) occurred:

* aws_autoscaling_group.masters: Error creating AutoScaling Group: ValidationError: Provided Load Balancers may not be valid. Please ensure they exist and try again.
	status code: 400, request id: b4eed151-9739-11e8-a465-6b34e899330a

retest this please

@wking
Copy link
Member Author

wking commented Aug 3, 2018

The Jenkins error was:

17:25:18         --- FAIL: Test/Cluster/AllResourcesCreated (600.00s)
17:25:18         	cluster_test.go:215: looking for resources defined by the provided manifests...
17:25:18         	cluster_test.go:252: MISSING : [Kind: /v1:Secret | Namespace: kube-system | Name: coreos-pull-secret] - [manifests/pull.json]
17:25:18         	cluster_test.go:252: MISSING : [Kind: /v1:Secret | Namespace: tectonic-ingress | Name: coreos-pull-secret] - [tectonic/ingress/pull.json]
17:25:18         	cluster_test.go:252: MISSING : [Kind: /v1:Secret | Namespace: tectonic-system | Name: tectonic-license-secret] - [tectonic/secrets/license.json]
17:25:18         	cluster_test.go:252: MISSING : [Kind: /v1:Secret | Namespace: tectonic-system | Name: coreos-pull-secret] - [tectonic/secrets/pull.json]
17:25:18         	cluster_test.go:265: invalid character 'a' looking for beginning of value
...
17:25:18         	smoke_test.go:112: failed with error: all defined resources were not present
17:25:18         	smoke_test.go:113: retrying in 30s
17:25:18         	cluster_test.go:289: timed out waiting for all manifests to be created after 10m0s

Looks like I still have some work to do on rerolling the manifest detection.

@openshift-bot
Copy link
Contributor

@wking: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 17, 2018
@crawford
Copy link
Contributor

/close

@openshift-ci-robot
Copy link
Contributor

@crawford: Closing this PR.

Details

In response to this:

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

stbenjam pushed a commit to stbenjam/installer that referenced this pull request Feb 10, 2021
…enshift-4.7-ose-cluster-baremetal-operator

Updating ose-cluster-baremetal-operator builder & base images to be consistent with ART
clnperez pushed a commit to clnperez/installer that referenced this pull request Jan 31, 2022
Currently we don't set ImageBucketFileName and use a hardcoded image. This
should be configurable but also require no user input by default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants