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

ci: Update CNS daemonset capabilities within E2E #2902

Merged
merged 1 commit into from
Aug 10, 2024

Conversation

jpayne3506
Copy link
Contributor

Reason for Change:

Updates CNS daemonset capabilities to ensure that a race condition does not occur when creating the /var/run/azure-vnet directory

Issue Fixed:

Contributes to CI/CD to cover #2818

Requirements:

Notes:

@jpayne3506 jpayne3506 requested a review from a team as a code owner August 7, 2024 00:22
@jpayne3506 jpayne3506 requested a review from jshr-w August 7, 2024 00:22
Copy link
Member

@timraymond timraymond left a comment

Choose a reason for hiding this comment

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

This looks fine to me, but I'm a little unclear as to how dropping capabilities prevents a race condition on creating a directory. Can you explain in a comment before you merge?

@rbtr
Copy link
Contributor

rbtr commented Aug 9, 2024

It is not preventing a race, it is testing that we no longer have a race between the CNI and kubelet creating the dirs with incompatible permissions. CNI used to make it with 0644, but kubelet would make it with 0755 - so if CNI wins, CNS should not have access to the dir.
...but it does, because it has CAP_DAC_OVERRIDE by default. Dropping caps exposes the race and ensures we have resolved it by changing the CNI Mkdir perms.
This was a fun one to debug 🥲

@rbtr
Copy link
Contributor

rbtr commented Aug 9, 2024

fixes #2818

@timraymond
Copy link
Member

@rbtr Awesome, that makes sense to me :)

@jpayne3506
Copy link
Contributor Author

Thanks for the explanation @rbtr, was going to link the issue and ask for you to comment more if needed 😆 .

@jpayne3506 jpayne3506 added this pull request to the merge queue Aug 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 9, 2024
@rbtr rbtr added this pull request to the merge queue Aug 9, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 9, 2024
@jpayne3506 jpayne3506 added this pull request to the merge queue Aug 9, 2024
Merged via the queue into master with commit 032890a Aug 10, 2024
93 checks passed
@jpayne3506 jpayne3506 deleted the jpayne3506/cns-drop branch August 10, 2024 02:00
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.

3 participants