Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Sedef committed Jul 30, 2020
1 parent b504826 commit 07cfd79
Show file tree
Hide file tree
Showing 4 changed files with 112 additions and 49 deletions.
29 changes: 26 additions & 3 deletions exp/addons/controllers/clusterresourceset_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"sigs.k8s.io/cluster-api/util/predicates"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
"sigs.k8s.io/controller-runtime/pkg/controller"
"sigs.k8s.io/controller-runtime/pkg/handler"
"sigs.k8s.io/controller-runtime/pkg/source"
Expand Down Expand Up @@ -95,7 +96,7 @@ func (r *ClusterResourceSetReconciler) SetupWithManager(mgr ctrl.Manager, option
&handler.EnqueueRequestsFromMapFunc{
ToRequests: handler.ToRequestsFunc(r.resourceToClusterResourceSet),
},
predicates.SecretCreateUpdate(r.Log),
predicates.AddonsSecretCreateUpdate(r.Log),
)
if err != nil {
return errors.Wrap(err, "failed adding Watch for Secret to controller manager")
Expand Down Expand Up @@ -417,14 +418,36 @@ func (r *ClusterResourceSetReconciler) clusterToClusterResourceSet(o handler.Map
func (r *ClusterResourceSetReconciler) resourceToClusterResourceSet(o handler.MapObject) []ctrl.Request {
result := []ctrl.Request{}

// Add all ClusterResourceSet owners.
for _, owner := range o.Meta.GetOwnerReferences() {
if owner.Kind == "ClusterResourceSet" {
name := client.ObjectKey{Namespace: o.Meta.GetNamespace(), Name: owner.Name}
result = append(result, ctrl.Request{NamespacedName: name})
}
}

// If there is any ClusterResourceSet owner, that means the resource is reconciled before,
// and existing owners are the only matching ClusterResourceSets to this resource, so no need to return all ClusterResourceSets.
if len(result) > 0 {
return result
}

// Only core group is accepted as resources group
if o.Object.GetObjectKind().GroupVersionKind().Group != "" {
return result
}

crsList := &addonsv1.ClusterResourceSetList{}
if err := r.Client.List(context.Background(), crsList, client.InNamespace(o.Meta.GetNamespace())); err != nil {
return nil
}

objKind, err := apiutil.GVKForObject(o.Object, r.scheme)
if err != nil {
return nil
}
for _, crs := range crsList.Items {
for _, resource := range crs.Spec.Resources {
if resource.Kind == o.Object.GetObjectKind().GroupVersionKind().Kind && crs.Name == o.Meta.GetName() {
if resource.Kind == objKind.Kind && resource.Name == o.Meta.GetName() {
name := client.ObjectKey{Namespace: o.Meta.GetNamespace(), Name: crs.Name}
result = append(result, ctrl.Request{NamespacedName: name})
break
Expand Down
75 changes: 75 additions & 0 deletions exp/addons/controllers/clusterresourceset_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,4 +217,79 @@ apiVersion: v1`,
return apierrors.IsNotFound(err)
}, timeout).Should(BeTrue())
})
It("Should reconcile a ClusterResourceSet when a resource is created that is part of ClusterResourceSet resources", func() {

labels := map[string]string{"foo2": "bar2"}
newCMName := "test-configmap2"

clusterResourceSetInstance := &addonsv1.ClusterResourceSet{
ObjectMeta: metav1.ObjectMeta{
Name: "test-clusterresourceset",
Namespace: defaultNamespaceName,
},
Spec: addonsv1.ClusterResourceSetSpec{
ClusterSelector: metav1.LabelSelector{
MatchLabels: labels,
},
Resources: []addonsv1.ResourceRef{{Name: newCMName, Kind: "ConfigMap"}},
},
}
// Create the ClusterResourceSet.
Expect(testEnv.Create(ctx, clusterResourceSetInstance)).To(Succeed())
defer func() {
Expect(testEnv.Delete(ctx, clusterResourceSetInstance)).To(Succeed())
}()

testCluster.SetLabels(labels)
Expect(testEnv.Update(ctx, testCluster)).To(Succeed())

By("Verifying ClusterResourceSetBinding is created with cluster owner reference")
// Wait until ClusterResourceSetBinding is created for the Cluster
clusterResourceSetBindingKey := client.ObjectKey{
Namespace: testCluster.Namespace,
Name: testCluster.Name,
}
Eventually(func() bool {
binding := &addonsv1.ClusterResourceSetBinding{}

err := testEnv.Get(ctx, clusterResourceSetBindingKey, binding)
return err == nil
}, timeout).Should(BeTrue())

// Initially ConfiMap is missing, so no resources in the binding.
Eventually(func() bool {
binding := &addonsv1.ClusterResourceSetBinding{}

err := testEnv.Get(ctx, clusterResourceSetBindingKey, binding)
if err == nil {
if len(binding.Spec.Bindings) > 0 && len(binding.Spec.Bindings[0].Resources) == 0 {
return true
}
}
return false
}, timeout).Should(BeTrue())

testConfigmap := &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Name: newCMName,
Namespace: defaultNamespaceName,
},
Data: map[string]string{},
}
Expect(testEnv.Create(ctx, testConfigmap)).To(Succeed())

// When the ConfigMap resource is created, CRS should get reconciled immediately.
Eventually(func() bool {
binding := &addonsv1.ClusterResourceSetBinding{}

err := testEnv.Get(ctx, clusterResourceSetBindingKey, binding)
if err == nil {
if len(binding.Spec.Bindings[0].Resources) > 0 && binding.Spec.Bindings[0].Resources[0].Name == newCMName {
return true
}
}
return false
}, timeout).Should(BeTrue())
Expect(testEnv.Delete(ctx, testConfigmap)).To(Succeed())
})
})
30 changes: 2 additions & 28 deletions util/predicates/configmap_predicates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,41 +18,15 @@ package predicates

import (
"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)

// ConfigMapCreate returns a predicate that returns true for a ConfigMap create event
func ConfigMapCreateUpdate(logger logr.Logger) predicate.Funcs {
log := logger.WithValues("predicate", "ConfigMapCreateOrUpdate")
return predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
log = log.WithValues("eventType", "create")

c, ok := e.Object.(*corev1.ConfigMap)
if !ok {

log.V(4).Info("Expected ConfigMap", "type", e.Object.GetObjectKind().GroupVersionKind().String())
return false
}
log = log.WithValues("namespace", c.Namespace, "configmap", c.Name)

return true
},
UpdateFunc: func(e event.UpdateEvent) bool {
log = log.WithValues("eventType", "update")

_, okOld := e.ObjectOld.(*corev1.ConfigMap)
newConfigMap, okNew := e.ObjectNew.(*corev1.ConfigMap)
if !okOld || !okNew {
log.V(4).Info("Expected ConfigMap", "type", e.ObjectOld.GetObjectKind().GroupVersionKind().String())
return false
}
log = log.WithValues("namespace", newConfigMap.Namespace, "configmap", newConfigMap.Name)

return true
},
CreateFunc: func(e event.CreateEvent) bool { return true },
UpdateFunc: func(e event.UpdateEvent) bool { return false },
DeleteFunc: func(e event.DeleteEvent) bool { return false },
GenericFunc: func(e event.GenericEvent) bool { return false },
}
Expand Down
27 changes: 9 additions & 18 deletions util/predicates/secret_predicates.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,40 +19,31 @@ package predicates
import (
"github.com/go-logr/logr"
corev1 "k8s.io/api/core/v1"
addonsv1 "sigs.k8s.io/cluster-api/exp/addons/api/v1alpha3"
"sigs.k8s.io/controller-runtime/pkg/event"
"sigs.k8s.io/controller-runtime/pkg/predicate"
)

// SecretCreateUpdate returns a predicate that returns true for a Secret create event
func SecretCreateUpdate(logger logr.Logger) predicate.Funcs {
// AddonsSecretCreateUpdate returns a predicate that returns true for a Secret create event if in addons Secret type
func AddonsSecretCreateUpdate(logger logr.Logger) predicate.Funcs {
log := logger.WithValues("predicate", "SecretCreateOrUpdate")

return predicate.Funcs{
CreateFunc: func(e event.CreateEvent) bool {
log = log.WithValues("eventType", "create")

c, ok := e.Object.(*corev1.Secret)
s, ok := e.Object.(*corev1.Secret)
if !ok {
log.V(4).Info("Expected Secret", "type", e.Object.GetObjectKind().GroupVersionKind().String())
log.V(4).Info("Expected Secret", "secret", e.Object.GetObjectKind().GroupVersionKind().String())
return false
}
log = log.WithValues("namespace", c.Namespace, "secret", c.Name)
log.V(4).Info("Expected Secret")
return true
},
UpdateFunc: func(e event.UpdateEvent) bool {
log = log.WithValues("eventType", "update")

_, okOld := e.ObjectOld.(*corev1.Secret)
newSecret, okNew := e.ObjectNew.(*corev1.Secret)
if !okOld || !okNew {
log.V(4).Info("Expected Secret", "type", e.ObjectOld.GetObjectKind().GroupVersionKind().String())
if string(s.Type) != string(addonsv1.ClusterResourceSetSecretType) {
log.V(4).Info("Expected Secret Type", "type", addonsv1.SecretClusterResourceSetResourceKind,
"got", string(s.Type))
return false
}
log = log.WithValues("namespace", newSecret.Namespace, "secret", newSecret.Name)

return true
},
UpdateFunc: func(e event.UpdateEvent) bool { return false },
DeleteFunc: func(e event.DeleteEvent) bool { return false },
GenericFunc: func(e event.GenericEvent) bool { return false },
}
Expand Down

0 comments on commit 07cfd79

Please sign in to comment.