Step 2 – controlplane/apiserver: move peer proxy code to allow generic aggregator construction #124576
+122
−71
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
release-note-none
sttts
force-pushed
the
sttts-peer-proxy-generic-move
branch
from
7527c49
to
4077c3a
Compare
k8s-ci-robot
added
area/apiserver
sig/api-machinery
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sttts 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 |
k8s-ci-robot
added
the
approved
sttts
added
the
priority/important-soon
k8s-ci-robot
removed
the
needs-priority
sttts
changed the title
sttts
changed the title
sttts
force-pushed
the
sttts-peer-proxy-generic-move
branch
from
4077c3a
to
4ad30af
Compare
|
/retest |
|
/lgtm |
k8s-ci-robot
added
the
do-not-merge/hold
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: 032d48615f43dc58a2d3e8a8d422b2b4111a59e7
|
richabanker
reviewed
Apr 29, 2024
pkg/controlplane/instance.go
Outdated
| @@ -290,6 +275,11 @@ func (c *Config) createEndpointReconciler() reconcilers.EndpointReconciler { | |||
|
|
|||
| // Complete fills in any fields not set that are required to have valid data. It's mutating the receiver. | |||
| func (c *Config) Complete() CompletedConfig { | |||
| if c.ControlPlane.PeerEndpointReconcileInterval == 0 && c.EndpointReconcilerConfig.Interval != 0 { | |||
| // default before the generic controlplane completion can | |||
|
/lgtm |
…truction This reverts commit 0bb6ff5e454cfc99c0b611e22162391eaa31e103. Signed-off-by: Dr. Stefan Schimanski <stefan.schimanski@gmail.com>
sttts
force-pushed
the
sttts-peer-proxy-generic-move
branch
from
4ad30af
to
c252ebe
Compare
k8s-ci-robot
removed
the
lgtm
|
/hold cancel |
k8s-ci-robot
removed
the
do-not-merge/hold
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: e0c3922d84c835d06732a607efdbc54c0212a47f
|
|
/retest |
luxas
reviewed
Apr 29, 2024
| CertFile: proxyClientCertFile, | ||
| KeyFile: proxyClientKeyFile, | ||
| CAFile: peerCAFile, | ||
| ServerName: "kubernetes.default.svc", |
There was a problem hiding this comment.
should this leak into generic or be an option?
There was a problem hiding this comment.
An option in follow-up (step 3).
| serializer, | ||
| ), nil | ||
| } | ||
|
|
||
| // utility function to get the apiserver address that is used by peer apiservers to proxy | ||
| // requests to this apiserver in case the peer is incapable of serving the request | ||
| func getPeerAddress(peerAdvertiseAddress peerreconcilers.PeerAdvertiseAddress, publicAddress net.IP, publicServicePort int) string { |
There was a problem hiding this comment.
is this kubeapi-specific code or not?
There was a problem hiding this comment.
Generic. It will be moved in #120202.
| // that can not be served locally | ||
| PeerProxy utilpeerproxy.Interface | ||
| // PeerEndpointReconcileInterval defines how often the endpoint leases are reconciled in etcd. | ||
| PeerEndpointReconcileInterval time.Duration |
There was a problem hiding this comment.
where is this used, if anywhere? it seems net new?
There was a problem hiding this comment.
It's defaulted in the generic code, and overridden with the service endpoint reconciler interval (= old behaviour) for kube-apiserver.
|
/triage accepted |
k8s-ci-robot
added
triage/accepted
sttts
changed the title
sttts
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Move peer proxy config flags and constructors to
pkg/controlplane/apiserverto make it consumable by a generic controlplane.Part of #124530.
Note that this reverts some moves from #124551 intentionally (!), to make this changed logic easy to review.
Which issue(s) this PR fixes:
Towards kubernetes/enhancements#4080.
Fixes #
Special notes for your reviewer:
This is purely a code move with exception of defaulting the peer pendpoint reconciler interval
DefaultPeerEndpointReconcileIntervalin the generic code path, but setting it toc.EndpointReconcilerConfig.Intervalas before this PR for kube-apiserver. Hence, the behaviour should be unchanged.Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: