From a402a05fbbee8660b8128eacf0164b3df3f9472e Mon Sep 17 00:00:00 2001 From: Enxebre Date: Tue, 22 Oct 2019 10:27:51 +0200 Subject: [PATCH] Only reconcile owned deployments --- pkg/operator/baremetal_pod.go | 3 + pkg/operator/operator.go | 70 +++++++++++++++++++++-- pkg/operator/operator_test.go | 101 +++++++++++++++++++++++++--------- pkg/operator/sync.go | 3 + 4 files changed, 146 insertions(+), 31 deletions(-) diff --git a/pkg/operator/baremetal_pod.go b/pkg/operator/baremetal_pod.go index a2531bfe91..78dad99d68 100644 --- a/pkg/operator/baremetal_pod.go +++ b/pkg/operator/baremetal_pod.go @@ -106,6 +106,9 @@ func newMetal3Deployment(config *OperatorConfig) *appsv1.Deployment { ObjectMeta: metav1.ObjectMeta{ Name: "metal3", Namespace: config.TargetNamespace, + Annotations: map[string]string{ + maoOwnedAnnotation: "", + }, Labels: map[string]string{ "api": "clusterapi", "k8s-app": "controller", diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index 34fb498ec3..79f1a4913a 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -28,7 +28,8 @@ const ( // a machineconfig pool is going to be requeued: // // 5ms, 10ms, 20ms, 40ms, 80ms, 160ms, 320ms, 640ms, 1.3s, 2.6s, 5.1s, 10.2s, 20.4s, 41s, 82s - maxRetries = 15 + maxRetries = 15 + maoOwnedAnnotation = "machine.openshift.io/owned" ) // Operator defines machine api operator. @@ -88,7 +89,7 @@ func New( operandVersions: operandVersions, } - deployInformer.Informer().AddEventHandler(optr.eventHandler()) + deployInformer.Informer().AddEventHandler(optr.eventHandlerDeployments()) featureGateInformer.Informer().AddEventHandler(optr.eventHandler()) optr.config = config @@ -125,13 +126,72 @@ func (optr *Operator) Run(workers int, stopCh <-chan struct{}) { <-stopCh } +func logResource(obj interface{}) { + metaObj, okObject := obj.(metav1.Object) + if !okObject { + glog.Errorf("Error assigning type to interface when logging") + } + glog.V(4).Infof("Resource type: %T", obj) + glog.V(4).Infof("Resource: %v", metaObj.GetSelfLink()) +} + func (optr *Operator) eventHandler() cache.ResourceEventHandler { workQueueKey := fmt.Sprintf("%s/%s", optr.namespace, optr.name) return cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) { optr.queue.Add(workQueueKey) }, - UpdateFunc: func(old, new interface{}) { optr.queue.Add(workQueueKey) }, - DeleteFunc: func(obj interface{}) { optr.queue.Add(workQueueKey) }, + AddFunc: func(obj interface{}) { + glog.V(4).Infof("Event: Add") + logResource(obj) + optr.queue.Add(workQueueKey) + }, + UpdateFunc: func(old, new interface{}) { + glog.V(4).Infof("Event: Update") + logResource(old) + optr.queue.Add(workQueueKey) + }, + DeleteFunc: func(obj interface{}) { + glog.V(4).Infof("Event: Delete") + logResource(obj) + optr.queue.Add(workQueueKey) + }, + } +} + +// on deployments we only reconcile on update/delete events if its owned by mao +func (optr *Operator) eventHandlerDeployments() cache.ResourceEventHandler { + workQueueKey := fmt.Sprintf("%s/%s", optr.namespace, optr.name) + return cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + glog.V(4).Infof("Event: Add") + logResource(obj) + optr.queue.Add(workQueueKey) + }, + UpdateFunc: func(old, new interface{}) { + glog.V(4).Infof("Event: Update") + logResource(old) + if owned, err := isOwned(old); !owned || err != nil { + return + } + optr.queue.Add(workQueueKey) + }, + DeleteFunc: func(obj interface{}) { + glog.V(4).Infof("Event: Delete") + logResource(obj) + if owned, err := isOwned(obj); !owned || err != nil { + return + } + optr.queue.Add(workQueueKey) + }, + } +} + +func isOwned(obj interface{}) (bool, error) { + metaObj, okObject := obj.(metav1.Object) + if !okObject { + glog.Errorf("Error assigning metav1.Object type to object: %T", obj) + return false, fmt.Errorf("error assigning metav1.Object type to object: %T", obj) } + _, ok := metaObj.GetAnnotations()[maoOwnedAnnotation] + return ok, nil } func (optr *Operator) worker() { diff --git a/pkg/operator/operator_test.go b/pkg/operator/operator_test.go index 35bb5fa551..ee7cf67491 100644 --- a/pkg/operator/operator_test.go +++ b/pkg/operator/operator_test.go @@ -5,6 +5,11 @@ import ( "testing" "time" + openshiftv1 "github.com/openshift/api/config/v1" + fakeos "github.com/openshift/client-go/config/clientset/versioned/fake" + configinformersv1 "github.com/openshift/client-go/config/informers/externalversions" + "github.com/stretchr/testify/assert" + appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -12,11 +17,6 @@ import ( fakekube "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" - - v1 "github.com/openshift/api/config/v1" - fakeos "github.com/openshift/client-go/config/clientset/versioned/fake" - configinformersv1 "github.com/openshift/client-go/config/informers/externalversions" - "github.com/stretchr/testify/assert" ) const ( @@ -25,12 +25,12 @@ const ( hcControllerName = "machine-healthcheck-controller" ) -func newFeatureGate(featureSet v1.FeatureSet) *v1.FeatureGate { - return &v1.FeatureGate{ +func newFeatureGate(featureSet openshiftv1.FeatureSet) *openshiftv1.FeatureGate { + return &openshiftv1.FeatureGate{ ObjectMeta: metav1.ObjectMeta{ Name: MachineAPIFeatureGateName, }, - Spec: v1.FeatureGateSpec{ + Spec: openshiftv1.FeatureGateSpec{ FeatureSet: featureSet, }, } @@ -75,7 +75,7 @@ func newFakeOperator(kubeObjects []runtime.Object, osObjects []runtime.Object, s kubeNamespacedSharedInformer.Start(stopCh) optr.syncHandler = optr.sync - deployInformer.Informer().AddEventHandler(optr.eventHandler()) + deployInformer.Informer().AddEventHandler(optr.eventHandlerDeployments()) featureGateInformer.Informer().AddEventHandler(optr.eventHandler()) return optr @@ -85,31 +85,31 @@ func newFakeOperator(kubeObjects []runtime.Object, osObjects []runtime.Object, s // for platforms that are no-ops. func TestOperatorSync_NoOp(t *testing.T) { cases := []struct { - platform v1.PlatformType + platform openshiftv1.PlatformType expectedNoop bool }{ { - platform: v1.AWSPlatformType, + platform: openshiftv1.AWSPlatformType, expectedNoop: false, }, { - platform: v1.LibvirtPlatformType, + platform: openshiftv1.LibvirtPlatformType, expectedNoop: false, }, { - platform: v1.OpenStackPlatformType, + platform: openshiftv1.OpenStackPlatformType, expectedNoop: false, }, { - platform: v1.AzurePlatformType, + platform: openshiftv1.AzurePlatformType, expectedNoop: false, }, { - platform: v1.BareMetalPlatformType, + platform: openshiftv1.BareMetalPlatformType, expectedNoop: false, }, { - platform: v1.GCPPlatformType, + platform: openshiftv1.GCPPlatformType, expectedNoop: false, }, { @@ -117,11 +117,11 @@ func TestOperatorSync_NoOp(t *testing.T) { expectedNoop: false, }, { - platform: v1.VSpherePlatformType, + platform: openshiftv1.VSpherePlatformType, expectedNoop: true, }, { - platform: v1.NonePlatformType, + platform: openshiftv1.NonePlatformType, expectedNoop: true, }, { @@ -132,17 +132,17 @@ func TestOperatorSync_NoOp(t *testing.T) { for _, tc := range cases { t.Run(string(tc.platform), func(t *testing.T) { - infra := &v1.Infrastructure{ + infra := &openshiftv1.Infrastructure{ ObjectMeta: metav1.ObjectMeta{ Name: "cluster", }, - Status: v1.InfrastructureStatus{ + Status: openshiftv1.InfrastructureStatus{ Platform: tc.platform, }, } stopCh := make(<-chan struct{}) - optr := newFakeOperator(nil, []runtime.Object{newFeatureGate(v1.TechPreviewNoUpgrade), infra}, stopCh) + optr := newFakeOperator(nil, []runtime.Object{newFeatureGate(openshiftv1.TechPreviewNoUpgrade), infra}, stopCh) go optr.Run(2, stopCh) err := wait.PollImmediate(1*time.Second, 5*time.Second, func() (bool, error) { @@ -162,11 +162,11 @@ func TestOperatorSync_NoOp(t *testing.T) { if !assert.NoError(t, err, "failed to get clusteroperator") { t.Fatal() } - expectedConditions := map[v1.ClusterStatusConditionType]v1.ConditionStatus{ - v1.OperatorAvailable: v1.ConditionTrue, - v1.OperatorProgressing: v1.ConditionFalse, - v1.OperatorDegraded: v1.ConditionFalse, - v1.OperatorUpgradeable: v1.ConditionTrue, + expectedConditions := map[openshiftv1.ClusterStatusConditionType]openshiftv1.ConditionStatus{ + openshiftv1.OperatorAvailable: openshiftv1.ConditionTrue, + openshiftv1.OperatorProgressing: openshiftv1.ConditionFalse, + openshiftv1.OperatorDegraded: openshiftv1.ConditionFalse, + openshiftv1.OperatorUpgradeable: openshiftv1.ConditionTrue, } for _, c := range o.Status.Conditions { assert.Equal(t, expectedConditions[c.Type], c.Status, fmt.Sprintf("unexpected clusteroperator condition %s status", c.Type)) @@ -174,3 +174,52 @@ func TestOperatorSync_NoOp(t *testing.T) { }) } } + +func TestIsOwned(t *testing.T) { + testCases := []struct { + testCase string + obj interface{} + expected bool + expectedError bool + }{ + { + testCase: "with maoOwnedAnnotation returns true", + obj: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + maoOwnedAnnotation: "", + }, + }, + }, + expected: true, + }, + { + testCase: "with no maoOwnedAnnotation returns false", + obj: &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "any": "", + }, + }, + }, + expected: false, + }, + { + testCase: "bad type object returns error", + obj: "bad object", + expected: false, + expectedError: true, + }, + } + for _, tc := range testCases { + t.Run(string(tc.testCase), func(t *testing.T) { + got, err := isOwned(tc.obj) + if got != tc.expected { + t.Errorf("Expected: %v, got: %v", tc.expected, got) + } + if tc.expectedError != (err != nil) { + t.Errorf("ExpectedError: %v, got: %v", tc.expectedError, err) + } + }) + } +} diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 5da5d0337f..f1c10b20ab 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -154,6 +154,9 @@ func newDeployment(config *OperatorConfig, features map[string]bool) *appsv1.Dep ObjectMeta: metav1.ObjectMeta{ Name: "machine-api-controllers", Namespace: config.TargetNamespace, + Annotations: map[string]string{ + maoOwnedAnnotation: "", + }, Labels: map[string]string{ "api": "clusterapi", "k8s-app": "controller",