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

extensions lib: Consider dropping EnsureKubeAPIServerService as it is no longer required after ManagedIstio/APIServerSNI is unconditionally enabled #9755

Closed
ialidzhikov opened this issue May 15, 2024 · 0 comments · Fixed by #9770
Assignees
Labels
area/scalability Scalability related kind/cleanup Something that is not needed anymore and can be cleaned up

Comments

@ialidzhikov
Copy link
Member

ialidzhikov commented May 15, 2024

How to categorize this issue?

/area scalability
/kind cleanup

What would you like to be added:
Looking into #9020 and the generic Mutator I see that the EnsureKubeAPIServerService func from the Ensurer interface is not implemented for any of the provider extensions under github.com/gardener.

This is the only mutation for an object of kind Service:

switch x := new.(type) {
case *corev1.Service:
switch x.Name {
case v1beta1constants.DeploymentNameKubeAPIServer:
var oldSvc *corev1.Service
if old != nil {
var ok bool
oldSvc, ok = old.(*corev1.Service)
if !ok {
return errors.New("could not cast old object to corev1.Service")
}
}
extensionswebhook.LogMutation(m.logger, x.Kind, x.Namespace, x.Name)
return m.ensurer.EnsureKubeAPIServerService(ctx, gctx, x, oldSvc)
}

In the times before ManagedIstio/APIServerSNI when the kube-apiserver Service was of type LoadBalancer this extension point was used to add load balancer specific annotations to the kube-apiserver Service. Examples from the past:

After ManagedIstio/APIServerSNI are unconditionally enabled, these funcs are removed.
Today, these extension webhooks mutate Services but actually don't do anything in this mutation.
For example for provider-aws the controlplaneexposure webhook looks like:

  name: controlplaneexposure.aws.extensions.gardener.cloud
  namespaceSelector:
    matchExpressions:
    - key: seed.gardener.cloud/provider
      operator: In
      values:
      - aws
  objectSelector: {}
  reinvocationPolicy: Never
  rules:
  - apiGroups:
    - ""
    apiVersions:
    - v1
    operations:
    - CREATE
    - UPDATE
    resources:
    - services
    scope: '*'
  - apiGroups:
    - apps
    apiVersions:
    - v1
    operations:
    - CREATE
    - UPDATE
    resources:
    - deployments
    scope: '*'
  - apiGroups:
    - druid.gardener.cloud
    apiVersions:
    - v1alpha1
    operations:
    - CREATE
    - UPDATE
    resources:
    - etcds
    scope: '*'
  sideEffects: None
  timeoutSeconds: 10

while the webhook only needs to mutate ETCDs: https://github.com/gardener/gardener-extension-provider-aws/blob/aaf1ad53dfb45a14c63662f36611bb239f32bc2d/pkg/webhook/controlplaneexposure/ensurer.go#L35-L52

This means that every Service/Deployment create/update operation for Service is intercepted by the provider-aws webhook and the provider-aws webhook does not perform any mutation these Services/Deployments.
In cases where the ManagedSeed control plane and data plane are located in different continents, the latency of such webhook calls can be >100ms.

Istio Service annotations can be applied via the Seed spec:

# loadBalancerServices:
# annotations:
# foo: bar
# externalTrafficPolicy: Local
# zones:
# - name: europe-1a
# annotations:
# foo: bar
# externalTrafficPolicy: Local

Why is this needed:
See above.

@gardener-prow gardener-prow bot added the kind/cleanup Something that is not needed anymore and can be cleaned up label May 15, 2024
@ialidzhikov ialidzhikov added the area/scalability Scalability related label May 15, 2024
@gardener gardener deleted a comment from gardener-prow bot May 15, 2024
@ialidzhikov ialidzhikov self-assigned this May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/scalability Scalability related kind/cleanup Something that is not needed anymore and can be cleaned up
Projects
None yet
1 participant