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

Update awsmachine providerID and instanceID immediately after ec2:Run… #493

Conversation

Patryk-Stefanski
Copy link

This mitigates issues caused by falling back to tag-based searching for EC2 instances in case future AWS calls fail within CreateInstance(), such as attaching ENIs to security groups or tagging ENIs.

Ref kubernetes-sigs/#4670

…Instances is called

This mitigates issues caused by falling back to tag-based searching for
EC2 instances in case future AWS calls fail, such as attaching ENIs to
security groups or tagging ENIs.

Signed-off-by: Michael Shen <[email protected]>
Copy link

openshift-ci bot commented Jan 11, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign enxebre for approval. For more information see the Kubernetes Code Review Process.

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

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

@JoelSpeed
Copy link

@damdo When are we next rebasing? Will this commit be included in the rebase?

@damdo
Copy link
Member

damdo commented Jan 11, 2024

@JoelSpeed I asked Richard if we can add them to the next minor and patch releases.

@damdo
Copy link
Member

damdo commented Jan 11, 2024

As soon as one or the other are out, I'll make sure to downstream them through a rebase

@Patryk-Stefanski
Copy link
Author

@damdo any estimate on when this will make it into the downstream?

@Patryk-Stefanski
Copy link
Author

/test verify-commits

Copy link

openshift-ci bot commented Feb 7, 2024

@Patryk-Stefanski: all tests passed!

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

@damdo
Copy link
Member

damdo commented Feb 7, 2024

@Patryk-Stefanski We are planning the next CAPA upstream release (v2.4.0) to be next week. Then it'll take a day or two to downstream it.

@Patryk-Stefanski
Copy link
Author

@damdo did this make it into the downstream?

@damdo
Copy link
Member

damdo commented Mar 4, 2024

It released upstream, we are in the process of downstreaming it here #499

@damdo
Copy link
Member

damdo commented Mar 6, 2024

@Patryk-Stefanski we merged the PR to downstream CAPA 2.4 which should contain this fix.

Let me know if it is there as expected, so we can close this PR. Thanks!

@Patryk-Stefanski
Copy link
Author

@damdo we are seeing issues that need this fix more prominently, whats the best way to go about having this also backported to 4.15 and 4.14?

@damdo
Copy link
Member

damdo commented Mar 18, 2024

/cherry-pick release-4.15

@openshift-cherrypick-robot

@damdo: cannot cherry-pick an unmerged PR

In response to this:

/cherry-pick release-4.15

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.

@damdo
Copy link
Member

damdo commented Mar 18, 2024

@Patryk-Stefanski we normally don't bump upstream patch releases to earlier release branches.
So best to open a similar PR like this one, against 4.15 (alongside an OCPBUG for it) then (/cherry-pick release-4.14) to backport it to 4.14.

Please remeber to follow the UPSTREAM: <carry>: commit message here format for the commit message of your backport. So the bots we use for the automated rebases behave correctly. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants