feat(olm): attempt to cleanup namespace annotations on shutdown

cmd/olm/main.go

@@ -103,6 +103,7 @@ func main() {
 	if err != nil {
 		log.Fatalf("error configuring operator: %s", err.Error())
 	}
+	defer operator.Cleanup()
 
 	// Serve a health check.
 	http.HandleFunc("/healthz", func(w http.ResponseWriter, r *http.Request) {

pkg/controller/annotator/annotator.go

@@ -24,7 +24,7 @@ func NewAnnotator(opClient operatorclient.ClientInterface, annotations map[strin
 	}
 }
 
-// AnnotateNamespaces takes a list of namespace names and a list of annotations to add to them
+// AnnotateNamespaces takes a list of namespace names and adds annotations to them
 func (a *Annotator) AnnotateNamespaces(namespaceNames []string) error {
 	if a.Annotations == nil {
 		return nil
@@ -44,6 +44,26 @@ func (a *Annotator) AnnotateNamespaces(namespaceNames []string) error {
 	return nil
 }
 
+// CleanNamespaceAnnotations takes a list of namespace names and removes annotations from them
+func (a *Annotator) CleanNamespaceAnnotations(namespaceNames []string) error {
+	if a.Annotations == nil {
+		return nil
+	}
+
+	namespaces, err := a.getNamespaces(namespaceNames)
+	if err != nil {
+		return err
+	}
+
+	for _, n := range namespaces {
+		if err := a.CleanNamespaceAnnotation(&n); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // getNamespaces gets the set of Namespace API objects given a list of names
 // if NamespaceAll is passed (""), all namespaces will be returned
 func (a *Annotator) getNamespaces(namespaceNames []string) (namespaces []corev1.Namespace, err error) {
@@ -97,3 +117,46 @@ func (a *Annotator) AnnotateNamespace(namespace *corev1.Namespace) error {
 	}
 	return nil
 }
+
+func (a *Annotator) CleanNamespaceAnnotation(namespace *corev1.Namespace) error {
+	originalName := namespace.GetName()
+	originalData, err := json.Marshal(namespace)
+	if err != nil {
+		return err
+	}
+
+	if namespace.Annotations == nil {
+		namespace.Annotations = map[string]string{}
+	}
+
+	annotations := map[string]string{}
+	for k, v := range namespace.Annotations {
+		annotations[k] = v
+	}
+
+	for key, value := range a.Annotations {
+		if existing, ok := namespace.Annotations[key]; ok && existing != value {
+			return fmt.Errorf("attempted to clean annotation %s:%s from namespace %s, but found unexpected annotation %s:%s", key, value, namespace.Name, key, existing)
+		} else if !ok {
+			// no namespace key to remove
+			return nil
+		}
+		delete(annotations, key)
+	}
+	namespace.SetAnnotations(annotations)
+
+	modifiedData, err := json.Marshal(namespace)
+	if err != nil {
+		return err
+	}
+	patchBytes, err := strategicpatch.CreateTwoWayMergePatch(originalData, modifiedData, corev1.Namespace{})
+	if err != nil {
+		return fmt.Errorf("error creating patch for Namespace: %v", err)
+	}
+	fmt.Println(string(patchBytes))
+	_, err = a.OpClient.KubernetesInterface().CoreV1().Namespaces().Patch(originalName, types.StrategicMergePatchType, patchBytes)
+	if err != nil {
+		return err
+	}
+	return nil
+}

pkg/controller/annotator/annotator_test.go

@@ -163,6 +163,51 @@ func TestAnnotateNamespace(t *testing.T) {
 				return
 			}
 			require.NoError(t, err)
+			fromCluster, err := fakeKubernetesClient.CoreV1().Namespaces().Get(namespace.Name, metav1.GetOptions{})
+			require.NoError(t, err)
+			require.Equal(t, tt.out, fromCluster.Annotations)
+		})
+	}
+}
+
+func TestCleanNamespaceAnnotation(t *testing.T) {
+	tests := []struct {
+		in          map[string]string
+		annotations map[string]string
+		out         map[string]string
+		errString   string
+		description string
+	}{
+		{
+			in:          map[string]string{"my": "annotation", "another": "annotation"},
+			annotations: map[string]string{"my": "annotation"},
+			out:         map[string]string{"another": "annotation"},
+			description: "CleanAnnotation",
+		},
+		{
+			in:          map[string]string{"my": "already-set"},
+			annotations: map[string]string{"my": "annotation"},
+			errString:   "attempted to clean annotation my:annotation from namespace ns, but found unexpected annotation my:already-set",
+			description: "AlreadyAnnotated",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.description, func(t *testing.T) {
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			namespace := namespaceObj("ns", tt.in)
+			mockClient, fakeKubernetesClient := NewMockNamespaceClient(ctrl, []corev1.Namespace{namespace})
+			if tt.errString == "" {
+				mockClient.EXPECT().KubernetesInterface().Return(fakeKubernetesClient)
+			}
+			annotator := NewAnnotator(mockClient, tt.annotations)
+			err := annotator.CleanNamespaceAnnotation(&namespace)
+			if tt.errString != "" {
+				require.EqualError(t, err, tt.errString)
+				return
+			}
+			require.NoError(t, err)
 			// hack because patch on the kubernetes fake doesn't seem to work
 			fakeKubernetesClient.CoreV1().Namespaces().Update(&namespace)
 			fromCluster, err := fakeKubernetesClient.CoreV1().Namespaces().Get(namespace.Name, metav1.GetOptions{})
@@ -217,6 +262,62 @@ func TestAnnotateNamespaces(t *testing.T) {
 			annotator := NewAnnotator(mockClient, tt.inAnnotations)
 			err := annotator.AnnotateNamespaces(tt.inNamespaces)
 
+			if tt.errString != "" {
+				require.EqualError(t, err, tt.errString)
+				return
+			}
+			require.NoError(t, err)
+			for _, namespaceName := range tt.inNamespaces {
+				fromCluster, err := fakeKubernetesClient.CoreV1().Namespaces().Get(namespaceName, metav1.GetOptions{})
+				require.NoError(t, err)
+				for _, expected := range tt.outNamespaces {
+					if expected.Name == fromCluster.Name {
+						require.Equal(t, expected.Annotations, fromCluster.Annotations)
+					}
+				}
+			}
+		})
+	}
+}
+
+func TestCleanNamespaceAnnotations(t *testing.T) {
+	tests := []struct {
+		inNamespaces       []string
+		inAnnotations      map[string]string
+		outNamespaces      []corev1.Namespace
+		existingNamespaces []corev1.Namespace
+		errString          string
+		description        string
+	}{
+		{
+			inNamespaces:       []string{"ns1"},
+			inAnnotations:      map[string]string{"my": "annotation"},
+			existingNamespaces: []corev1.Namespace{namespaceObj("ns1", map[string]string{"my": "annotation", "existing": "note"})},
+			outNamespaces:      []corev1.Namespace{namespaceObj("ns1", map[string]string{"existing": "note"})},
+			description:        "CleanAnnotation",
+		},
+		{
+			inNamespaces:       []string{"ns1"},
+			inAnnotations:      map[string]string{"my": "annotation"},
+			existingNamespaces: []corev1.Namespace{namespaceObj("ns1", map[string]string{"my": "already-set"})},
+			errString:          "attempted to clean annotation my:annotation from namespace ns1, but found unexpected annotation my:already-set",
+			description:        "AlreadyAnnotated",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.description, func(t *testing.T) {
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			mockClient, fakeKubernetesClient := NewMockNamespaceClient(ctrl, tt.existingNamespaces)
+			mockClient.EXPECT().KubernetesInterface().Return(fakeKubernetesClient)
+			if tt.errString == "" {
+				mockClient.EXPECT().KubernetesInterface().Return(fakeKubernetesClient)
+			}
+
+			annotator := NewAnnotator(mockClient, tt.inAnnotations)
+			err := annotator.CleanNamespaceAnnotations(tt.inNamespaces)
+
 			if tt.errString != "" {
 				require.EqualError(t, err, tt.errString)
 				return

pkg/controller/operators/olm/operator.go

@@ -32,10 +32,11 @@ const (
 
 type Operator struct {
 	*queueinformer.Operator
-	csvQueue  workqueue.RateLimitingInterface
-	client    versioned.Interface
-	resolver  install.StrategyResolverInterface
-	annotator *annotator.Annotator
+	csvQueue    workqueue.RateLimitingInterface
+	client      versioned.Interface
+	resolver    install.StrategyResolverInterface
+	annotator   *annotator.Annotator
+	cleanupFunc func()
 }
 
 func NewOperator(crClient versioned.Interface, opClient operatorclient.ClientInterface, resolver install.StrategyResolverInterface, wakeupInterval time.Duration, annotations map[string]string, namespaces []string) (*Operator, error) {
@@ -57,6 +58,9 @@ func NewOperator(crClient versioned.Interface, opClient operatorclient.ClientInt
 		client:    crClient,
 		resolver:  resolver,
 		annotator: namespaceAnnotator,
+		cleanupFunc: func() {
+			namespaceAnnotator.CleanNamespaceAnnotations(namespaces)
+		},
 	}
 
 	// if watching all namespaces, set up a watch to annotate new namespaces
@@ -124,6 +128,10 @@ func NewOperator(crClient versioned.Interface, opClient operatorclient.ClientInt
 	return op, nil
 }
 
+func (a *Operator) Cleanup() {
+	a.cleanupFunc()
+}
+
 func (a *Operator) requeueCSV(name, namespace string) {
 	// we can build the key directly, will need to change if queue uses different key scheme
 	key := fmt.Sprintf("%s/%s", namespace, name)