Skip to content

Conversation

squeed
Copy link
Contributor

@squeed squeed commented Sep 26, 2022

The script originally just exited and assumed the kubelet would restart it, but this is quite disruptive, since that restarts the agent as well.

Additionally, it was discovered that some AWS installations need the service network to be functioning before the AWS CNI file is written, leaving clusters in a broken state.

So, change the script to retry looking for the AWS CNI file itself.

Fixes: #21243

Fixes cilium startup on certain AWS-VPC clusters.

@squeed squeed added kind/bug This is a bug in the Cilium logic. area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. release-note/bug This PR fixes an issue in a previous release of Cilium. labels Sep 26, 2022
@squeed squeed requested a review from a team as a code owner September 26, 2022 09:03
@squeed squeed requested a review from sayboras September 26, 2022 09:03
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

LGTM ✔️

@squeed
Copy link
Contributor Author

squeed commented Sep 28, 2022

/test
not sure what happened to Jenkins

@sayboras
Copy link
Member

sayboras commented Sep 30, 2022

/test

Job 'Cilium-PR-K8s-1.25-kernel-net-next' failed:

Click to show.

Test Name

K8sDatapathConfig Check BPF masquerading with ip-masq-agent VXLAN

Failure Output

FAIL: Failed to add ip route

If it is a flake and a GitHub issue doesn't already exist to track it, comment /mlh new-flake Cilium-PR-K8s-1.25-kernel-net-next so I can create one.

@squeed squeed added the dont-merge/preview-only Only for preview or testing, don't merge it. label Oct 4, 2022
@squeed
Copy link
Contributor Author

squeed commented Oct 4, 2022

Ah interesting, AWS-cni failed. Something isn't right; marking as do-not-merge.

The script originally just exited and assumed the kubelet would restart
it, but this is quite disruptive, since that restarts the agent as well.

Additionally, it was discovered that some AWS installations need the
service network to be functioning before the AWS CNI file is written,
leaving clusters in a broken state.

So, change the script to retry looking for the AWS CNI file itself.

Fixes: cilium#21243

Signed-off-by: Casey Callendrello <cdc@isovalent.com>
@squeed
Copy link
Contributor Author

squeed commented Oct 4, 2022

Oops, issue fixed. find never fails to flummox me.

Thanks, CI! You're the best.

@squeed squeed added needs-backport/1.12 and removed dont-merge/preview-only Only for preview or testing, don't merge it. labels Oct 5, 2022
@squeed
Copy link
Contributor Author

squeed commented Oct 5, 2022

/ci-awscni

@squeed
Copy link
Contributor Author

squeed commented Oct 5, 2022

/ci-eks

@squeed
Copy link
Contributor Author

squeed commented Oct 5, 2022

ci-ekscni and ci-aws passed. This is ready to merge.

@sayboras sayboras added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Oct 5, 2022
@ldelossa ldelossa merged commit d45999d into cilium:master Oct 6, 2022
@sayboras sayboras mentioned this pull request Oct 8, 2022
8 tasks
@sayboras sayboras added backport-pending/1.12 backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. and removed needs-backport/1.12 labels Oct 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cni Impacts the Container Networking Interface between Cilium and the orchestrator. backport-done/1.12 The backport for Cilium 1.12.x for this PR is done. kind/bug This is a bug in the Cilium logic. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/bug This PR fixes an issue in a previous release of Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AWS CNI config not generated upgrading from 1.11.8 to 1.12.1
3 participants