Skip to content

Commit 288451f

Browse files
tjamettjamet
committed
Make all controllers to watch secrets
As requested in the reviews, change the reconcile logic and remove the added secret reconciler. Instead, all generic reconcilers will listen to secret changes and trigger a reconciliation for each Provider using the secret
1 parent 7488ec9 commit 288451f

8 files changed

+342
-326
lines changed

‎cmd/main.go

Lines changed: 35 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ import (
4646
operatorv1alpha1 "sigs.k8s.io/cluster-api-operator/api/v1alpha1"
4747
operatorv1 "sigs.k8s.io/cluster-api-operator/api/v1alpha2"
4848
providercontroller "sigs.k8s.io/cluster-api-operator/internal/controller"
49-
"sigs.k8s.io/cluster-api-operator/internal/controller/genericprovider"
5049
healtchcheckcontroller "sigs.k8s.io/cluster-api-operator/internal/controller/healthcheck"
5150
)
5251

@@ -216,70 +215,77 @@ func setupChecks(mgr ctrl.Manager) {
216215

217216
func setupReconcilers(mgr ctrl.Manager, watchConfigSecretChanges bool) {
218217
if err := (&providercontroller.GenericProviderReconciler{
219-
Provider: &operatorv1.CoreProvider{},
220-
ProviderList: &operatorv1.CoreProviderList{},
221-
Client: mgr.GetClient(),
222-
Config: mgr.GetConfig(),
218+
Provider: &operatorv1.CoreProvider{},
219+
ProviderList: &operatorv1.CoreProviderList{},
220+
Client: mgr.GetClient(),
221+
Config: mgr.GetConfig(),
222+
WatchConfigSecretChanges: watchConfigSecretChanges,
223223
}).SetupWithManager(mgr, concurrency(concurrencyNumber)); err != nil {
224224
setupLog.Error(err, "unable to create controller", "controller", "CoreProvider")
225225
os.Exit(1)
226226
}
227227

228228
if err := (&providercontroller.GenericProviderReconciler{
229-
Provider: &operatorv1.InfrastructureProvider{},
230-
ProviderList: &operatorv1.InfrastructureProviderList{},
231-
Client: mgr.GetClient(),
232-
Config: mgr.GetConfig(),
229+
Provider: &operatorv1.InfrastructureProvider{},
230+
ProviderList: &operatorv1.InfrastructureProviderList{},
231+
Client: mgr.GetClient(),
232+
Config: mgr.GetConfig(),
233+
WatchConfigSecretChanges: watchConfigSecretChanges,
233234
}).SetupWithManager(mgr, concurrency(concurrencyNumber)); err != nil {
234235
setupLog.Error(err, "unable to create controller", "controller", "InfrastructureProvider")
235236
os.Exit(1)
236237
}
237238

238239
if err := (&providercontroller.GenericProviderReconciler{
239-
Provider: &operatorv1.BootstrapProvider{},
240-
ProviderList: &operatorv1.BootstrapProviderList{},
241-
Client: mgr.GetClient(),
242-
Config: mgr.GetConfig(),
240+
Provider: &operatorv1.BootstrapProvider{},
241+
ProviderList: &operatorv1.BootstrapProviderList{},
242+
Client: mgr.GetClient(),
243+
Config: mgr.GetConfig(),
244+
WatchConfigSecretChanges: watchConfigSecretChanges,
243245
}).SetupWithManager(mgr, concurrency(concurrencyNumber)); err != nil {
244246
setupLog.Error(err, "unable to create controller", "controller", "BootstrapProvider")
245247
os.Exit(1)
246248
}
247249

248250
if err := (&providercontroller.GenericProviderReconciler{
249-
Provider: &operatorv1.ControlPlaneProvider{},
250-
ProviderList: &operatorv1.ControlPlaneProviderList{},
251-
Client: mgr.GetClient(),
252-
Config: mgr.GetConfig(),
251+
Provider: &operatorv1.ControlPlaneProvider{},
252+
ProviderList: &operatorv1.ControlPlaneProviderList{},
253+
Client: mgr.GetClient(),
254+
Config: mgr.GetConfig(),
255+
WatchConfigSecretChanges: watchConfigSecretChanges,
253256
}).SetupWithManager(mgr, concurrency(concurrencyNumber)); err != nil {
254257
setupLog.Error(err, "unable to create controller", "controller", "ControlPlaneProvider")
255258
os.Exit(1)
256259
}
257260

258261
if err := (&providercontroller.GenericProviderReconciler{
259-
Provider: &operatorv1.AddonProvider{},
260-
ProviderList: &operatorv1.AddonProviderList{},
261-
Client: mgr.GetClient(),
262-
Config: mgr.GetConfig(),
262+
Provider: &operatorv1.AddonProvider{},
263+
ProviderList: &operatorv1.AddonProviderList{},
264+
Client: mgr.GetClient(),
265+
Config: mgr.GetConfig(),
266+
WatchConfigSecretChanges: watchConfigSecretChanges,
263267
}).SetupWithManager(mgr, concurrency(concurrencyNumber)); err != nil {
264268
setupLog.Error(err, "unable to create controller", "controller", "AddonProvider")
265269
os.Exit(1)
266270
}
267271

268272
if err := (&providercontroller.GenericProviderReconciler{
269-
Provider: &operatorv1.IPAMProvider{},
270-
ProviderList: &operatorv1.IPAMProviderList{},
271-
Client: mgr.GetClient(),
272-
Config: mgr.GetConfig(),
273+
Provider: &operatorv1.IPAMProvider{},
274+
ProviderList: &operatorv1.IPAMProviderList{},
275+
Client: mgr.GetClient(),
276+
Config: mgr.GetConfig(),
277+
WatchConfigSecretChanges: watchConfigSecretChanges,
273278
}).SetupWithManager(mgr, concurrency(concurrencyNumber)); err != nil {
274279
setupLog.Error(err, "unable to create controller", "controller", "IPAMProvider")
275280
os.Exit(1)
276281
}
277282

278283
if err := (&providercontroller.GenericProviderReconciler{
279-
Provider: &operatorv1.RuntimeExtensionProvider{},
280-
ProviderList: &operatorv1.RuntimeExtensionProviderList{},
281-
Client: mgr.GetClient(),
282-
Config: mgr.GetConfig(),
284+
Provider: &operatorv1.RuntimeExtensionProvider{},
285+
ProviderList: &operatorv1.RuntimeExtensionProviderList{},
286+
Client: mgr.GetClient(),
287+
Config: mgr.GetConfig(),
288+
WatchConfigSecretChanges: watchConfigSecretChanges,
283289
}).SetupWithManager(mgr, concurrency(concurrencyNumber)); err != nil {
284290
setupLog.Error(err, "unable to create controller", "controller", "RuntimeExtensionProvider")
285291
os.Exit(1)
@@ -291,24 +297,6 @@ func setupReconcilers(mgr ctrl.Manager, watchConfigSecretChanges bool) {
291297
setupLog.Error(err, "unable to create controller", "controller", "Healthcheck")
292298
os.Exit(1)
293299
}
294-
295-
if watchConfigSecretChanges {
296-
if err := (&providercontroller.SecretReconciler{
297-
ProviderLists: []genericprovider.GenericProviderList{
298-
&operatorv1.CoreProviderList{},
299-
&operatorv1.InfrastructureProviderList{},
300-
&operatorv1.BootstrapProviderList{},
301-
&operatorv1.ControlPlaneProviderList{},
302-
&operatorv1.AddonProviderList{},
303-
&operatorv1.IPAMProviderList{},
304-
&operatorv1.RuntimeExtensionProviderList{},
305-
},
306-
Client: mgr.GetClient(),
307-
}).SetupWithManager(mgr, concurrency(concurrencyNumber)); err != nil {
308-
setupLog.Error(err, "unable to create controller", "controller", "Secret")
309-
os.Exit(1)
310-
}
311-
}
312300
}
313301

314302
func setupWebhooks(mgr ctrl.Manager) {

‎internal/controller/genericprovider_controller.go

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
"errors"
2424
"fmt"
2525

26-
v1 "k8s.io/api/core/v1"
26+
corev1 "k8s.io/api/core/v1"
2727
apierrors "k8s.io/apimachinery/pkg/api/errors"
2828
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2929
kerrors "k8s.io/apimachinery/pkg/util/errors"
@@ -37,14 +37,16 @@ import (
3737
"sigs.k8s.io/controller-runtime/pkg/client"
3838
"sigs.k8s.io/controller-runtime/pkg/controller"
3939
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
40+
"sigs.k8s.io/controller-runtime/pkg/handler"
4041
"sigs.k8s.io/controller-runtime/pkg/reconcile"
4142
)
4243

4344
type GenericProviderReconciler struct {
44-
Provider genericprovider.GenericProvider
45-
ProviderList genericprovider.GenericProviderList
46-
Client client.Client
47-
Config *rest.Config
45+
Provider genericprovider.GenericProvider
46+
ProviderList genericprovider.GenericProviderList
47+
Client client.Client
48+
Config *rest.Config
49+
WatchConfigSecretChanges bool
4850
}
4951

5052
const (
@@ -53,9 +55,16 @@ const (
5355
)
5456

5557
func (r *GenericProviderReconciler) SetupWithManager(mgr ctrl.Manager, options controller.Options) error {
56-
return ctrl.NewControllerManagedBy(mgr).
57-
For(r.Provider).
58-
WithOptions(options).
58+
builder := ctrl.NewControllerManagedBy(mgr).
59+
For(r.Provider)
60+
if r.WatchConfigSecretChanges {
61+
builder.Watches(
62+
&corev1.Secret{},
63+
handler.EnqueueRequestsFromMapFunc(newSecretToProviderFuncMapForProviderList(r.Client, r.ProviderList)),
64+
)
65+
}
66+
67+
return builder.WithOptions(options).
5968
Complete(r)
6069
}
6170

@@ -244,7 +253,7 @@ func (r *GenericProviderReconciler) reconcileDelete(ctx context.Context, provide
244253

245254
func (r *GenericProviderReconciler) getProviderConfigSecretHash(ctx context.Context) (string, error) {
246255
if r.Provider.GetSpec().ConfigSecret != nil {
247-
secret := &v1.Secret{
256+
secret := &corev1.Secret{
248257
ObjectMeta: metav1.ObjectMeta{
249258
Namespace: r.Provider.GetSpec().ConfigSecret.Namespace,
250259
Name: r.Provider.GetSpec().ConfigSecret.Name,

‎internal/controller/genericprovider_controller_test.go

Lines changed: 115 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
. "github.com/onsi/gomega"
2424
appsv1 "k8s.io/api/apps/v1"
2525
corev1 "k8s.io/api/core/v1"
26+
v1 "k8s.io/api/core/v1"
2627
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
2728
"k8s.io/apimachinery/pkg/runtime"
2829
utilruntime "k8s.io/apimachinery/pkg/util/runtime"
@@ -43,13 +44,15 @@ releaseSeries:
4344
minor: 4
4445
contract: v1beta1
4546
`
46-
testComponents = `
47+
testDeploymentName = "capd-controller-manager"
48+
testComponents = `
4749
apiVersion: apps/v1
4850
kind: Deployment
4951
metadata:
5052
labels:
5153
cluster.x-k8s.io/provider: infrastructure-docker
5254
control-plane: controller-manager
55+
value-from-config: ${CONFIGURED_VALUE:=default-value}
5356
name: capd-controller-manager
5457
namespace: capd-system
5558
spec:
@@ -105,6 +108,107 @@ func dummyConfigMap(ns, name string) *corev1.ConfigMap {
105108
}
106109
}
107110

111+
func createDummyProviderWithConfigSecret(objs []client.Object, provider genericprovider.GenericProvider, configSecret *v1.Secret) ([]client.Object, error) {
112+
cm := dummyConfigMap(provider.GetNamespace(), testCurrentVersion)
113+
114+
if err := env.CreateAndWait(ctx, cm); err != nil {
115+
return objs, err
116+
}
117+
objs = append(objs, cm)
118+
provider.SetSpec(operatorv1.ProviderSpec{
119+
Version: testCurrentVersion,
120+
ConfigSecret: &operatorv1.SecretReference{
121+
Name: configSecret.GetName(),
122+
Namespace: configSecret.GetNamespace(),
123+
},
124+
ManifestPatches: []string{},
125+
})
126+
insertDummyConfig(provider)
127+
err := env.CreateAndWait(ctx, provider)
128+
if err != nil {
129+
return objs, err
130+
}
131+
objs = append(objs, provider)
132+
return objs, nil
133+
}
134+
135+
func testDeploymentLabelValueGetter(deploymentNS, deploymentName string) func() string {
136+
return func() string {
137+
deployment := &appsv1.Deployment{
138+
ObjectMeta: metav1.ObjectMeta{
139+
Name: deploymentName,
140+
Namespace: deploymentNS,
141+
},
142+
}
143+
144+
if err := env.Get(ctx, client.ObjectKeyFromObject(deployment), deployment); err != nil {
145+
return ""
146+
}
147+
148+
return deployment.Labels["value-from-config"]
149+
}
150+
}
151+
152+
func TestConfigSecretChangesAreAppliedTotheDeployment(t *testing.T) {
153+
g := NewWithT(t)
154+
objs := []client.Object{}
155+
defer func() {
156+
g.Expect(env.CleanupAndWait(ctx, objs...)).To(Succeed())
157+
}()
158+
159+
ns, err := env.CreateNamespace(ctx, "config-secret-namespace")
160+
g.Expect(err).ToNot(HaveOccurred())
161+
g.Expect(ns.Name).NotTo(BeEmpty())
162+
163+
t.Log("Ensure namespace exists", ns.Name)
164+
165+
configSecret := &corev1.Secret{
166+
ObjectMeta: metav1.ObjectMeta{
167+
GenerateName: "config-secret-",
168+
Namespace: ns.Name,
169+
},
170+
StringData: map[string]string{
171+
"CONFIGURED_VALUE": "initial-value",
172+
},
173+
}
174+
g.Expect(env.CreateAndWait(ctx, configSecret)).To(Succeed())
175+
objs = append(objs, configSecret)
176+
177+
t.Log("Created config secret")
178+
179+
objs, err = createDummyProviderWithConfigSecret(
180+
objs,
181+
&operatorv1.CoreProvider{
182+
ObjectMeta: metav1.ObjectMeta{
183+
Name: "cluster-api",
184+
Namespace: ns.Name,
185+
},
186+
},
187+
configSecret,
188+
)
189+
g.Expect(err).To(Succeed())
190+
191+
t.Log("Created provider")
192+
193+
g.Eventually(
194+
testDeploymentLabelValueGetter(ns.Name, testDeploymentName),
195+
30*time.Second,
196+
).Should(BeEquivalentTo("initial-value"))
197+
198+
t.Log("Provider deploymnet deployed")
199+
200+
configSecret.Data["CONFIGURED_VALUE"] = []byte("updated-value")
201+
202+
g.Expect(env.Update(ctx, configSecret)).NotTo(HaveOccurred())
203+
204+
t.Log("Config secret updated")
205+
206+
g.Eventually(
207+
testDeploymentLabelValueGetter(ns.Name, testDeploymentName),
208+
30*time.Second,
209+
).Should(BeEquivalentTo("updated-value"))
210+
}
211+
108212
func TestReconcilerPreflightConditions(t *testing.T) {
109213
testCases := []struct {
110214
name string
@@ -379,7 +483,7 @@ releaseSeries:
379483
// Ensure customization occurred
380484
dep := &appsv1.Deployment{ObjectMeta: metav1.ObjectMeta{
381485
Namespace: provider.Namespace,
382-
Name: "capd-controller-manager",
486+
Name: testDeploymentName,
383487
}}
384488
if err := env.Get(ctx, client.ObjectKeyFromObject(dep), dep); err != nil {
385489
return false
@@ -460,6 +564,10 @@ func TestProviderConfigSecretChanges(t *testing.T) {
460564
for _, tc := range testCases {
461565
t.Run(tc.name, func(t *testing.T) {
462566
g := NewWithT(t)
567+
objs := []client.Object{}
568+
defer func() {
569+
g.Expect(env.CleanupAndWait(ctx, objs...)).To(Succeed())
570+
}()
463571

464572
dataHash, err := calculateHash(tc.cmData)
465573
g.Expect(err).ToNot(HaveOccurred())
@@ -504,7 +612,9 @@ func TestProviderConfigSecretChanges(t *testing.T) {
504612
t.Log("Ensure namespace exists", configNamespace.Name)
505613
g.Expect(err).ToNot(HaveOccurred())
506614

507-
g.Expect(env.CreateAndWait(ctx, dummyConfigMap(providerNamespace.Name, testCurrentVersion))).To(Succeed())
615+
cm := dummyConfigMap(providerNamespace.Name, testCurrentVersion)
616+
g.Expect(env.CreateAndWait(ctx, cm)).To(Succeed())
617+
objs = append(objs, cm)
508618

509619
provider.Namespace = providerNamespace.Name
510620
provider.Spec.ConfigSecret.Namespace = configNamespace.Name
@@ -518,9 +628,11 @@ func TestProviderConfigSecretChanges(t *testing.T) {
518628
}
519629

520630
g.Expect(env.CreateAndWait(ctx, secret.DeepCopy())).To(Succeed())
631+
objs = append(objs, secret)
521632

522633
t.Log("creating test provider", provider.GetName())
523634
g.Expect(env.CreateAndWait(ctx, provider.DeepCopy())).To(Succeed())
635+
objs = append(objs, provider)
524636

525637
g.Eventually(generateExpectedResultChecker(provider, appliedConfigHashAnnotation, dataHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
526638

@@ -553,9 +665,6 @@ func TestProviderConfigSecretChanges(t *testing.T) {
553665
}).Should(Succeed())
554666

555667
g.Eventually(generateExpectedResultChecker(provider, appliedConfigHashAnnotation, updatedDataHash, corev1.ConditionTrue), timeout).Should(BeEquivalentTo(true))
556-
557-
// Clean up
558-
g.Expect(env.Cleanup(ctx, provider, secret, providerNamespace, configNamespace)).To(Succeed())
559668
})
560669
}
561670
}

0 commit comments

Comments
 (0)