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

Fix issues with --disable-agent and --egress-selector-mode=pod|cluster #7331

Merged
merged 4 commits into from
Apr 28, 2023

Conversation

brandond
Copy link
Member

@brandond brandond commented Apr 20, 2023

Proposed Changes

Improve egress selector handling on agentless servers

  • Add an e2e test for agentless servers
  • Don't set up the agent tunnel authorizer on agentless servers, and warn when agentless servers won't have a way to reach in-cluster endpoints.
  • Fix a regression in the (non-default) cluster and pod egress-selector-modes

Types of Changes

bugfix

Verification

See linked issues

Testing

Yes, added one

Linked Issues

User-Facing Change

* Servers started with the (experimental) --disable-agent flag no longer attempt to run the tunnel authorizer agent component.
* Fixed an regression that prevented the pod and cluster egress-selector modes from working properly.

Further Comments

@codecov-commenter
Copy link

codecov-commenter commented Apr 21, 2023

Codecov Report

Patch coverage has no change and project coverage change: +9.52 🎉

Comparison is base (8d0255a) 9.82% compared to head (e23431b) 19.34%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7331      +/-   ##
==========================================
+ Coverage    9.82%   19.34%   +9.52%     
==========================================
  Files         147       81      -66     
  Lines       10845     5449    -5396     
==========================================
- Hits         1065     1054      -11     
+ Misses       9558     4173    -5385     
  Partials      222      222              
Flag Coverage Δ
unittests 19.34% <0.00%> (+9.52%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/agent/flannel/setup.go 0.00% <0.00%> (ø)
pkg/authenticator/passwordfile/passwordfile.go 80.64% <ø> (ø)
pkg/cli/cmds/etcd_snapshot.go 0.00% <0.00%> (ø)
pkg/cloudprovider/cloudprovider.go 0.00% <0.00%> (ø)
pkg/cluster/managed.go 0.00% <0.00%> (ø)
pkg/daemons/config/types.go 67.21% <ø> (ø)
pkg/daemons/executor/embed.go 2.08% <0.00%> (ø)

... and 66 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@brandond brandond marked this pull request as ready for review April 21, 2023 05:57
@brandond brandond requested a review from a team as a code owner April 21, 2023 05:57
@brandond brandond force-pushed the no-agentless-tunnel-auth branch from 76af32c to 80d68b4 Compare April 21, 2023 20:05
@brandond brandond changed the title Improve egress selector handling on agentless servers Fix issues with --disable-agent and --egress-selector-mode=pod|cluster Apr 21, 2023
Don't set up the agent tunnel authorizer on agentless servers, and warn when agentless servers won't have a way to reach in-cluster endpoints.

Signed-off-by: Brad Davidson <[email protected]>
tests/e2e/disableagent/disableagent_test.go Outdated Show resolved Hide resolved
tests/e2e/disableagent/Vagrantfile Outdated Show resolved Hide resolved
@brandond brandond force-pushed the no-agentless-tunnel-auth branch from 80d68b4 to e23431b Compare April 22, 2023 00:55
@brandond brandond requested review from dereknola and a team April 22, 2023 00:56
Several places in the code used a 5-second retry loop to wait on
Runtime.Core to be set. This caused a race condition where OnChange
handlers could be added after the Wrangler shared informers were already
started. When this happened, the handlers were never called because the
shared informers they relied upon were not started.

Fix that by requiring anything that waits on Runtime.Core to run from a
cluster controller startup hook that is guaranteed to be called before
the shared informers are started, instead of just firing it off in a
goroutine that retries until it is set.

Signed-off-by: Brad Davidson <[email protected]>
Signed-off-by: Brad Davidson <[email protected]>
@brandond brandond force-pushed the no-agentless-tunnel-auth branch from e23431b to 57b517f Compare April 24, 2023 20:45
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.

4 participants