Skip to content

Commit d2115d0

Browse files
vrutkovsvrutkovs
committed
nokubeconfig controller: use listers, filter informer events, avoid unecessary applies
1 parent 3b26613 commit d2115d0

File tree

2 files changed

+144
-19
lines changed

2 files changed

+144
-19
lines changed

‎pkg/operator/nodekubeconfigcontroller/nodekubeconfigcontroller.go‎

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"encoding/base64"
66
"fmt"
7+
"reflect"
78
"strings"
89
"time"
910

@@ -20,12 +21,17 @@ import (
2021
"github.com/openshift/library-go/pkg/operator/resource/resourceread"
2122
"github.com/openshift/library-go/pkg/operator/v1helpers"
2223
corev1 "k8s.io/api/core/v1"
24+
apierrors "k8s.io/apimachinery/pkg/api/errors"
2325
"k8s.io/client-go/kubernetes"
2426
coreclientv1 "k8s.io/client-go/kubernetes/typed/core/v1"
2527
corev1listers "k8s.io/client-go/listers/core/v1"
2628
)
2729

28-
const workQueueKey = "key"
30+
const (
31+
workQueueKey = "key"
32+
kubeApiserverServerCA = "kube-apiserver-server-ca"
33+
nodeSystemAdminClient = "node-system-admin-client"
34+
)
2935

3036
type NodeKubeconfigController struct {
3137
operatorClient v1helpers.StaticPodOperatorClient
@@ -40,24 +46,37 @@ func NewNodeKubeconfigController(
4046
operatorClient v1helpers.StaticPodOperatorClient,
4147
kubeInformersForNamespaces v1helpers.KubeInformersForNamespaces,
4248
kubeClient kubernetes.Interface,
43-
infrastuctureInformer configv1informers.InfrastructureInformer,
49+
infrastructureInformer configv1informers.InfrastructureInformer,
4450
eventRecorder events.Recorder,
4551
) factory.Controller {
4652
c := &NodeKubeconfigController{
4753
operatorClient: operatorClient,
4854
kubeClient: kubeClient,
4955
configMapLister: kubeInformersForNamespaces.ConfigMapLister(),
5056
secretLister: kubeInformersForNamespaces.SecretLister(),
51-
infrastructureLister: infrastuctureInformer.Lister(),
52-
}
53-
54-
return factory.New().WithInformers(
57+
infrastructureLister: infrastructureInformer.Lister(),
58+
}
59+
60+
return factory.New().WithFilteredEventsInformers(
61+
func(obj interface{}) bool {
62+
if cm, ok := obj.(*corev1.ConfigMap); ok {
63+
if cm.Namespace == operatorclient.TargetNamespace && cm.Name == kubeApiserverServerCA {
64+
return true
65+
}
66+
return false
67+
}
68+
if secret, ok := obj.(*corev1.Secret); ok {
69+
if secret.Namespace == operatorclient.OperatorNamespace && secret.Name == nodeSystemAdminClient {
70+
return true
71+
}
72+
return false
73+
}
74+
return true
75+
},
5576
operatorClient.Informer(),
56-
kubeInformersForNamespaces.InformersFor(operatorclient.OperatorNamespace).Core().V1().ConfigMaps().Informer(),
5777
kubeInformersForNamespaces.InformersFor(operatorclient.TargetNamespace).Core().V1().ConfigMaps().Informer(),
5878
kubeInformersForNamespaces.InformersFor(operatorclient.OperatorNamespace).Core().V1().Secrets().Informer(),
59-
kubeInformersForNamespaces.InformersFor(operatorclient.TargetNamespace).Core().V1().Secrets().Informer(),
60-
infrastuctureInformer.Informer(),
79+
infrastructureInformer.Informer(),
6180
).WithSync(c.sync).WithSyncDegradedOnError(c.operatorClient).ResyncEvery(5*time.Minute).ToController("NodeKubeconfigController", eventRecorder.WithComponentSuffix("node-kubeconfig-controller"))
6281
}
6382

@@ -99,27 +118,27 @@ func (c NodeKubeconfigController) sync(ctx context.Context, syncContext factory.
99118
func ensureNodeKubeconfigs(ctx context.Context, client coreclientv1.CoreV1Interface, secretLister corev1listers.SecretLister, configmapLister corev1listers.ConfigMapLister, infrastructureLister configv1listers.InfrastructureLister, recorder events.Recorder) error {
100119
requiredSecret := resourceread.ReadSecretV1OrDie(bindata.MustAsset("assets/kube-apiserver/node-kubeconfigs.yaml"))
101120

102-
systemAdminCredsSecret, err := secretLister.Secrets(operatorclient.OperatorNamespace).Get("node-system-admin-client")
121+
systemAdminCredsSecret, err := secretLister.Secrets(operatorclient.OperatorNamespace).Get(nodeSystemAdminClient)
103122
if err != nil {
104123
return err
105124
}
106125

107126
systemAdminClientCert := systemAdminCredsSecret.Data[corev1.TLSCertKey]
108127
if len(systemAdminClientCert) == 0 {
109-
return fmt.Errorf("system:admin client certificate missing from secret %s/node-system-admin-client", operatorclient.OperatorNamespace)
128+
return fmt.Errorf("system:admin client certificate missing from secret %s/%s", operatorclient.OperatorNamespace, nodeSystemAdminClient)
110129
}
111130
systemAdminClientKey := systemAdminCredsSecret.Data[corev1.TLSPrivateKeyKey]
112131
if len(systemAdminClientKey) == 0 {
113-
return fmt.Errorf("system:admin client private key missing from secret %s/node-system-admin-client", operatorclient.OperatorNamespace)
132+
return fmt.Errorf("system:admin client private key missing from secret %s/%s", operatorclient.OperatorNamespace, nodeSystemAdminClient)
114133
}
115134

116-
servingCABundleCM, err := configmapLister.ConfigMaps(operatorclient.TargetNamespace).Get("kube-apiserver-server-ca")
135+
servingCABundleCM, err := configmapLister.ConfigMaps(operatorclient.TargetNamespace).Get(kubeApiserverServerCA)
117136
if err != nil {
118137
return err
119138
}
120139
servingCABundleData := servingCABundleCM.Data["ca-bundle.crt"]
121140
if len(servingCABundleData) == 0 {
122-
return fmt.Errorf("serving CA bundle missing from configmap %s/kube-apiserver-server-ca", operatorclient.TargetNamespace)
141+
return fmt.Errorf("serving CA bundle missing from configmap %s/%s", operatorclient.TargetNamespace, kubeApiserverServerCA)
123142
}
124143

125144
infrastructure, err := infrastructureLister.Get("cluster")
@@ -161,10 +180,15 @@ func ensureNodeKubeconfigs(ctx context.Context, client coreclientv1.CoreV1Interf
161180
requiredSecret.Annotations[certrotation.CertificateNotAfterAnnotation] = systemAdminCredsSecret.Annotations[certrotation.CertificateNotAfterAnnotation]
162181
}
163182

164-
_, _, err = resourceapply.ApplySecret(ctx, client, recorder, requiredSecret)
165-
if err != nil {
166-
return err
183+
actualSecret, err := secretLister.Secrets(requiredSecret.Namespace).Get(requiredSecret.Name)
184+
if !apierrors.IsNotFound(err) {
185+
if err != nil {
186+
return err
187+
}
188+
if reflect.DeepEqual(actualSecret.Data, requiredSecret.Data) && reflect.DeepEqual(actualSecret.Annotations, requiredSecret.Annotations) {
189+
return nil
190+
}
167191
}
168-
169-
return nil
192+
_, _, err = resourceapply.ApplySecret(ctx, client, recorder, requiredSecret)
193+
return err
170194
}

‎pkg/operator/nodekubeconfigcontroller/nodekubeconfigcontroller_test.go‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,107 @@ func TestEnsureNodeKubeconfigs(t *testing.T) {
365365
},
366366
},
367367
},
368+
{
369+
name: "annotations only update",
370+
existingObjects: []runtime.Object{
371+
&corev1.ConfigMap{
372+
ObjectMeta: metav1.ObjectMeta{
373+
Namespace: "openshift-kube-apiserver",
374+
Name: "kube-apiserver-server-ca",
375+
},
376+
Data: map[string]string{
377+
"ca-bundle.crt": "kube-apiserver-server-ca certificate",
378+
},
379+
},
380+
&corev1.Secret{
381+
ObjectMeta: metav1.ObjectMeta{
382+
Namespace: "openshift-kube-apiserver-operator",
383+
Name: "node-system-admin-client",
384+
Annotations: map[string]string{
385+
certrotation.CertificateNotBeforeAnnotation: certNotBefore,
386+
certrotation.CertificateNotAfterAnnotation: certNotAfter,
387+
},
388+
},
389+
Data: map[string][]byte{
390+
"tls.crt": []byte(publicKey),
391+
"tls.key": []byte(privateKey),
392+
},
393+
},
394+
&corev1.Secret{
395+
TypeMeta: metav1.TypeMeta{
396+
APIVersion: "v1",
397+
Kind: "Secret",
398+
},
399+
ObjectMeta: metav1.ObjectMeta{
400+
Namespace: "openshift-kube-apiserver",
401+
Name: "node-kubeconfigs",
402+
Annotations: map[string]string{
403+
annotations.OpenShiftComponent: "kube-apiserver",
404+
certrotation.CertificateNotBeforeAnnotation: "some-old-not-before",
405+
certrotation.CertificateNotAfterAnnotation: "some-old-not-after",
406+
},
407+
},
408+
Data: map[string][]byte{
409+
"localhost.kubeconfig": generateKubeConfig("localhost", "https://localhost:6443"),
410+
"localhost-recovery.kubeconfig": generateKubeConfig("localhost-recovery", "https://localhost:6443"),
411+
"lb-ext.kubeconfig": generateKubeConfig("lb-ext", lbExtServer),
412+
"lb-int.kubeconfig": generateKubeConfig("lb-int", lbIntServer),
413+
},
414+
},
415+
},
416+
infrastructure: &configv1.Infrastructure{
417+
ObjectMeta: metav1.ObjectMeta{
418+
Namespace: "",
419+
Name: "cluster",
420+
},
421+
Status: configv1.InfrastructureStatus{
422+
APIServerURL: lbExtServer,
423+
APIServerInternalURL: lbIntServer,
424+
},
425+
},
426+
expectedErr: nil,
427+
expectedActions: []clienttesting.Action{
428+
clienttesting.DeleteActionImpl{
429+
ActionImpl: clienttesting.ActionImpl{
430+
Namespace: "openshift-kube-apiserver",
431+
Verb: "delete",
432+
Resource: corev1.SchemeGroupVersion.WithResource("secrets"),
433+
},
434+
Name: "node-kubeconfigs",
435+
},
436+
clienttesting.CreateActionImpl{
437+
ActionImpl: clienttesting.ActionImpl{
438+
Namespace: "openshift-kube-apiserver",
439+
Verb: "create",
440+
Resource: corev1.SchemeGroupVersion.WithResource("secrets"),
441+
},
442+
Object: &corev1.Secret{
443+
TypeMeta: metav1.TypeMeta{
444+
APIVersion: "v1",
445+
Kind: "Secret",
446+
},
447+
ObjectMeta: metav1.ObjectMeta{
448+
Namespace: "openshift-kube-apiserver",
449+
Name: "node-kubeconfigs",
450+
Labels: map[string]string{},
451+
OwnerReferences: []metav1.OwnerReference{},
452+
Annotations: map[string]string{
453+
annotations.OpenShiftComponent: "kube-apiserver",
454+
certrotation.CertificateNotBeforeAnnotation: certNotBefore,
455+
certrotation.CertificateNotAfterAnnotation: certNotAfter,
456+
},
457+
},
458+
Data: map[string][]byte{
459+
"localhost.kubeconfig": generateKubeConfig("localhost", "https://localhost:6443"),
460+
"localhost-recovery.kubeconfig": generateKubeConfig("localhost-recovery", "https://localhost:6443"),
461+
"lb-ext.kubeconfig": generateKubeConfig("lb-ext", lbExtServer),
462+
"lb-int.kubeconfig": generateKubeConfig("lb-int", lbIntServer),
463+
},
464+
Type: corev1.SecretTypeOpaque,
465+
},
466+
},
467+
},
468+
},
368469
}
369470
for _, tc := range tt {
370471
t.Run(tc.name, func(t *testing.T) {

0 commit comments

Comments
 (0)