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

Spotinst: Avoid unnecessary duplication of tasks #10630

Merged
merged 1 commit into from
Jan 22, 2021

Conversation

liranp
Copy link
Contributor

@liranp liranp commented Jan 21, 2021

This PR fixes the regression introduced by #9699 (reported by @povils): renaming the instance group results in nodeup failing to start with the following errors:

$ journalctl --no-pager -o cat -u kops-configuration
Starting Run kops bootstrap (nodeup)...
nodeup version 1.20.0-alpha.1
I0121 17:47:52.707955    1846 s3context.go:213] found bucket in region "us-west-2"
I0121 17:47:52.708041    1846 s3fs.go:290] Reading file "s3://<CONFIG_BASE>/cluster.spec"
I0121 17:47:52.742606    1846 s3fs.go:290] Reading file "s3://<CONFIG_BASE>/instancegroup/nodes.<CLUSTER_NAME>"
W0121 17:47:52.750671    1846 main.go:133] got error running nodeup (will retry in 30s): error loading InstanceGroup "s3://<CONFIG_BASE>/instancegroup/nodes.<CLUSTER_NAME>": file does not exist

@k8s-ci-robot k8s-ci-robot added do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 21, 2021
@k8s-ci-robot k8s-ci-robot added area/provider/spotinst Issues or PRs related to spotinst provider size/S Denotes a PR that changes 10-29 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 21, 2021
@hakman
Copy link
Member

hakman commented Jan 22, 2021

@liranp Can you edit the commit message?

@liranp
Copy link
Contributor Author

liranp commented Jan 22, 2021

Done it already. Sorry about that.

@liranp liranp changed the title Spotinst: Avoid unnecessary duplication of tasks (fixes #9699) Spotinst: Avoid unnecessary duplication of tasks Jan 22, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/invalid-commit-message Indicates that a PR should not merge because it has an invalid commit message. label Jan 22, 2021
@povils
Copy link

povils commented Jan 22, 2021

Thanks for picking this up! will it go to 1.19 ?

@hakman
Copy link
Member

hakman commented Jan 22, 2021

No worries, thanks!
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 22, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman, liranp

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit f97ef42 into kubernetes:master Jan 22, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.20 milestone Jan 22, 2021
@ReillyTevera
Copy link
Contributor

ReillyTevera commented Mar 6, 2021

@liranp @hakman This should be cherry-picked into 1.19 as this issue also affects that version. This will result in Ocean instances not starting up correctly as they try to retrieve the instance group specification located at ig.ObjectMeta.Name + "." + b.ClusterName() instead of the actual spec located at ig.ObjectMeta.Name. Obviously, as that file does not exist the node is not able to bootstrap further.

@hakman
Copy link
Member

hakman commented Mar 6, 2021

The cherry-pick was trivial. If @liranp thinks it's safe, I don't mind.

k8s-ci-robot added a commit that referenced this pull request Mar 6, 2021
…-upstream-release-1.19

Automated cherry pick of #10630: fix(spot/ocean): avoid unnecessary duplication of tasks
@liranp
Copy link
Contributor Author

liranp commented Mar 7, 2021

@ReillyTevera, thank you for your feedback. @hakman, I believe we can do that.

@liranp liranp deleted the fix-ocean-userdata branch March 7, 2021 09:40
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. area/provider/spotinst Issues or PRs related to spotinst provider cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants