-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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 1759617: vendor: update to use openshift fork of terraform-provider-aws #3621
Bug 1759617: vendor: update to use openshift fork of terraform-provider-aws #3621
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there is much point in switching forks before the fix has merged.
In #3603 (which is similar) I put a hold on the PR, point to my personal fork of the provider, and then ran that through CI. You could try that if helpful.
Also I have a suggestion after I benefited from reading a commit message like this: d983eae I try to include some details in the commit message whenever there is some external tooling or generated code used. I think it might help as I and perhaps other members on the team get more proficient with go modules.
9fb7b5a
to
b430add
Compare
/hold |
b430add
to
9aef341
Compare
@jhixson74: This pull request references Bugzilla bug 1815518, which is invalid:
Comment In response to this:
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. |
/hold cancel |
@jhixson74: This pull request references Bugzilla bug 1759617, which is invalid:
Comment In response to this:
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. |
9aef341
to
44b5511
Compare
/approve a small nit for easily seeing what we are replacing the vendor to, due to weirdness of psuedo versions from go mods. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya 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:
Approvers can indicate their approval by writing |
/bugzilla refresh |
@abhinavdahiya: This pull request references Bugzilla bug 1759617, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
In response to this:
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. |
This is to address the "NoSuchBucket: The specified bucket does not exist" error as explained in https://bugzilla.redhat.com/show_bug.cgi?id=1759617 and many other similar bugs. This bug has been "fixed" several times over the years, yet it continues to rear its ugly self. The ultimate problem is a race condition with S3 eventual consistency. As described in the bug above, the bucket does not yet exist when trying to reference tags. The openshift fork that this commit references, contains an upstream patch as described in hashicorp/terraform-provider-aws#13009 that should address this issue.
44b5511
to
4cd576b
Compare
/lgtm |
/retest Please review the full test history for this PR and help us cut down flakes. |
7 similar comments
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
/retest Please review the full test history for this PR and help us cut down flakes. |
@jhixson74: All pull requests linked via external trackers have merged: terraform-providers/terraform-provider-aws#12418, openshift/installer#3621, openshift/installer#3323, openshift/installer#2745. Bugzilla bug 1759617 has been moved to the MODIFIED state. In response to this:
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. |
This PR switches the terraform-provider-aws to use the OpenShift fork and updates to version v2.60.0 to address a number of bugs. The fork includes an upstream patch from a PR that is not yet committed.