Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug 1659970: terraform/exec/plugins/vendor: Bump terraform-provider-aws to v2.2.0 #1442

Merged

Conversation

wking
Copy link
Member

@wking wking commented Mar 20, 2019

This picks up a bunch of upstream improvements including hashicorp/terraform-provider-aws#7734 to address rhbz#1659970.

Generated with a manual Gopkg.toml edit followed by:

$ cd terraform/exec/plugins
$ dep ensure

with:

$ dep version
dep:
 version     : v0.5.0-31-g73b3afe
 build date  : 2019-02-08
 git hash    : 73b3afe
 go version  : go1.10.3
 go compiler : gc
 platform    : linux/amd64
 features    : ImportDuringSolve=false

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Mar 20, 2019
@@ -441,6 +552,55 @@
revision = "51d6538a90f86fe93ac480b35f37b2be17fef232"
version = "v2.2.2"

[[projects]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Ooo, this does not look right, how is pulling in k8s.io/apimachinery ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I dunno. I'll regenerate tonight with the debugging cranked up to see.


"github.com/hashicorp/terraform/helper/schema"
"github.com/hashicorp/terraform/helper/validation"
"github.com/kubernetes-sigs/aws-iam-authenticator/pkg/token"
Copy link
Member Author

Choose a reason for hiding this comment

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

@abhinavdahiya, here's where Kubernetes is getting pulled in, so I think this is not actually a problem.

This picks up a bunch of upstream improvements [1] including [2] to
address [3].

Generated with a manual Gopkg.toml edit followed by:

  $ cd terraform/exec/plugins
  $ dep ensure

with:

  $ dep version
  dep:
   version     : v0.5.1
   build date  : 2019-03-20
   git hash    : faa61893
   go version  : go1.10.3
   go compiler : gc
   platform    : linux/amd64
   features    : ImportDuringSolve=false

The aws-iam-authenticator pin gets us closer to [4], although (1) I
don't know why dep isn't figuring that out for itself and (2) they got
rid of their 0.3.1 tag [5]?  Anyway, this avoids:

  $ hack/build.sh
  ...
  # github.com/openshift/installer/pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws
  pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws/data_source_aws_eks_cluster_auth.go:35:38: too many arguments in call to token.NewGenerator
  have (bool)
  want ()
  ...

The AWS SDK pin gets us closer to [6] and avoids:

  $ hack/build.sh
  ...
  # github.com/openshift/installer/pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws
  pkg/terraform/exec/plugins/vendor/github.com/terraform-providers/terraform-provider-aws/aws/structure.go:4801:7: spec.ServiceNames undefined (type *appmesh.VirtualRouterSpec has no field or method ServiceNames)
  ...

The = prefixes in Gopkg.toml settle things down, because [7]:

  Note: When you specify a version without an operator, dep
  automatically uses the ^ operator by default. dep ensure will
  interpret the given version as the min-boundary of a range...

[1]: https://github.com/terraform-providers/terraform-provider-aws/blob/v2.2.0/CHANGELOG.md
[2]: hashicorp/terraform-provider-aws#7734 (v2.0.0)
[3]: https://bugzilla.redhat.com/show_bug.cgi?id=1659970
[4]: https://github.com/terraform-providers/terraform-provider-aws/blob/v2.2.0/go.mod#L27
[5]: https://github.com/kubernetes-sigs/aws-iam-authenticator/tags
[6]: https://github.com/terraform-providers/terraform-provider-aws/blob/v2.2.0/go.mod#L7
[7]: https://golang.github.io/dep/docs/Gopkg.toml.html#version-rules
@wking wking force-pushed the terraform-provider-aws-v2.2.0 branch from 6c2558b to d1c17b7 Compare March 26, 2019 22:55
@wking
Copy link
Member Author

wking commented Mar 26, 2019

I've pushed 6c2558b38 -> d1c17b7 adding two override stanzas and some = prefixes to Gopkg.toml to get this to compile. The AWS Terraform provider has a v2.3 out now, but I'll bump to that in follow-up work after we get this in.

@wking
Copy link
Member Author

wking commented Mar 27, 2019

Through Terraform:

Flaky tests:

[Feature:Builds][Conformance] s2i build with a quota  Building from a template should create an s2i build with a quota and run it [Suite:openshift/conformance/parallel/minimal]

Failing tests:

[sig-storage] In-tree Volumes [Driver: nfs] [Testpattern: Dynamic PV (default fs)] subPath should be able to unmount after the subpath directory is deleted [Suite:openshift/conformance/parallel] [Suite:k8s]
[sig-storage] In-tree Volumes [Driver: nfs] [Testpattern: Inline-volume (default fs)] subPath should be able to unmount after the subpath directory is deleted [Suite:openshift/conformance/parallel] [Suite:k8s]
[sig-storage] In-tree Volumes [Driver: nfs] [Testpattern: Pre-provisioned PV (default fs)] subPath should be able to unmount after the subpath directory is deleted [Suite:openshift/conformance/parallel] [Suite:k8s]

So I think we're good to go.

@abhinavdahiya
Copy link
Contributor

Through Terraform:

Flaky tests:

[Feature:Builds][Conformance] s2i build with a quota  Building from a template should create an s2i build with a quota and run it [Suite:openshift/conformance/parallel/minimal]

Failing tests:

[sig-storage] In-tree Volumes [Driver: nfs] [Testpattern: Dynamic PV (default fs)] subPath should be able to unmount after the subpath directory is deleted [Suite:openshift/conformance/parallel] [Suite:k8s]
[sig-storage] In-tree Volumes [Driver: nfs] [Testpattern: Inline-volume (default fs)] subPath should be able to unmount after the subpath directory is deleted [Suite:openshift/conformance/parallel] [Suite:k8s]
[sig-storage] In-tree Volumes [Driver: nfs] [Testpattern: Pre-provisioned PV (default fs)] subPath should be able to unmount after the subpath directory is deleted [Suite:openshift/conformance/parallel] [Suite:k8s]

So I think we're good to go.

do we need the 420K bump to fix BZ 1659970(IMO user error) 😢

@wking
Copy link
Member Author

wking commented Mar 27, 2019

do we need the 420K bump to fix BZ 1659970(IMO user error)

Lots of other bugfixes in there too, see the change-log. For example:

resource/aws_s3_bucket: Prevent NoSuchBucket errors when putting lifecycle configuration on resource creation (hashicorp/terraform-provider-aws#7930)

@wking
Copy link
Member Author

wking commented Mar 27, 2019

Failing tests are openshift/origin#22412, and may be happening every time.

@smarterclayton
Copy link
Contributor

/retest

@abhinavdahiya
Copy link
Contributor

do we need the 420K bump to fix BZ 1659970(IMO user error)

Lots of other bugfixes in there too, see the change-log. For example:

resource/aws_s3_bucket: Prevent NoSuchBucket errors when putting lifecycle configuration on resource creation (terraform-providers/terraform-provider-aws#7930)

That's not related to how we use that resource...

😢
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Mar 27, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, wking

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [abhinavdahiya,wking]

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

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@wking
Copy link
Member Author

wking commented Mar 27, 2019

e2e-aws:

level=error msg="\t* aws_route_table_association.private_routing.0: timeout while waiting for state to become 'success' (timeout: 5m0s)"

/retest

@sdodson
Copy link
Member

sdodson commented Mar 27, 2019

/retest

@sdodson
Copy link
Member

sdodson commented Mar 27, 2019

level=fatal msg="failed to initialize the cluster: Working towards 0.0.1-2019-03-27-195700: 99% complete: timed out waiting for the condition" on the last failure

@openshift-merge-robot openshift-merge-robot merged commit 42af12d into openshift:master Mar 28, 2019
@wking wking deleted the terraform-provider-aws-v2.2.0 branch March 28, 2019 01:04
wking added a commit to wking/openshift-installer that referenced this pull request Mar 28, 2019
…nfiguration

And alphabetize this section.  The new permission avoids [1]:

  error getting S3 Bucket Object Lock configuration: AccessDenied

which came in with our Terraform bump from d1c17b7
(terraform/exec/plugins/vendor: Bump terraform-provider-aws to v2.2.0,
2019-03-19, openshift#1442).

[1]: hashicorp/terraform-provider-aws#7550
wking added a commit to wking/openshift-installer that referenced this pull request Apr 1, 2019
We've been using the tagged private zone to look up the public zone
since the non-Terraform destroy code landed in a8fc89b (vendor: add
aws deprovision, openshift#324).  When there's an existing cluster with a given
domain, that can lead to false-positive removals like [1]:

1. Cluster 1 comes up like usual.
2. Cluster 2 creates a private zone.
3. Cluster 2 dies when its public record conflicts with cluster 1 (new
   since d1c17b7, terraform/exec/plugins/vendor: Bump
   terraform-provider-aws to v2.2.0, 2019-03-19, openshift#1442).
4. 'destroy cluster' on cluster 2's metadata.json removes cluster 2
   resources (good) and cluster 1's public record (bad).

With the explicit dependency in this commit, we ensure that we only
ever create the private zone after we have successfully claimed
ownership of the public record.

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1659970#c7
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. lgtm Indicates that a PR is ready to be merged. 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.

7 participants