diff --git a/controllers/async_controller.go b/controllers/async_controller.go index 29f6225aaa3..ea39cefd805 100644 --- a/controllers/async_controller.go +++ b/controllers/async_controller.go @@ -26,10 +26,11 @@ import ( ) const ( - finalizerName string = "azure.microsoft.com/finalizer" - requeueDuration time.Duration = time.Second * 20 - successMsg string = "successfully provisioned" - reconcileTimeout time.Duration = time.Minute * 5 + finalizerName string = "azure.microsoft.com/finalizer" + namespaceAnnotation string = "azure.microsoft.com/operator-namespace" + requeueDuration time.Duration = time.Second * 20 + successMsg string = "successfully provisioned" + reconcileTimeout time.Duration = time.Minute * 5 ) // AsyncReconciler is a generic reconciler for Azure resources. @@ -99,6 +100,31 @@ func (r *AsyncReconciler) Reconcile(req ctrl.Request, obj runtime.Object) (resul return ctrl.Result{}, r.Update(ctx, obj) } + // Ensure the resource is tagged with the operator's namespace. + reconcilerNamespace := annotations[namespaceAnnotation] + podNamespace := config.PodNamespace() + if reconcilerNamespace != podNamespace && reconcilerNamespace != "" { + // We don't want to get into a fight with another operator - + // so treat some other operator's annotation in a very similar + // way as the skip annotation above. This will do the right + // thing in the case of two operators trying to manage the + // same namespace. It makes moving objects between namespaces + // or changing which operator owns a namespace fiddlier (since + // you'd need to remove the annotation) but those operations + // are likely to be rare. + message := fmt.Sprintf("Operators in %q and %q are both configured to manage this resource", podNamespace, reconcilerNamespace) + r.Recorder.Event(obj, corev1.EventTypeWarning, "Overlap", message) + return ctrl.Result{}, r.Update(ctx, obj) + } else if reconcilerNamespace == "" { + // Set the annotation to this operator's namespace and go around again. + if annotations == nil { + annotations = make(map[string]string) + } + annotations[namespaceAnnotation] = podNamespace + res.SetAnnotations(annotations) + return ctrl.Result{}, r.Update(ctx, obj) + } + var configOptions []resourcemanager.ConfigOption if res.GetDeletionTimestamp().IsZero() { if !HasFinalizer(res, finalizerName) { diff --git a/controllers/targetnamespace_test.go b/controllers/targetnamespace_test.go index bee0f3d3235..89c79b8f738 100644 --- a/controllers/targetnamespace_test.go +++ b/controllers/targetnamespace_test.go @@ -14,7 +14,9 @@ import ( corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" v1alpha1 "github.com/Azure/azure-service-operator/api/v1alpha1" "github.com/Azure/azure-service-operator/pkg/helpers" @@ -57,6 +59,11 @@ func TestTargetNamespaces(t *testing.T) { EnsureInstance(ctx, t, tc, &instanceDefault) + require := require.New(t) + + // Check that the instance is annotated with the operator namespace. + checkNamespaceAnnotation(require, &instanceDefault, "azureoperator-system") + // The watched namespace is also reconciled. instanceWatched := instanceDefault instanceWatched.ObjectMeta = metav1.ObjectMeta{ @@ -65,26 +72,38 @@ func TestTargetNamespaces(t *testing.T) { } EnsureInstance(ctx, t, tc, &instanceWatched) + checkNamespaceAnnotation(require, &instanceWatched, "azureoperator-system") + // But the unwatched namespace isn't... + unwatchedName := newName() instanceUnwatched := instanceDefault instanceUnwatched.ObjectMeta = metav1.ObjectMeta{ - Name: newName(), + Name: unwatchedName, Namespace: "unwatched", } - require := require.New(t) err := tc.k8sClient.Create(ctx, &instanceUnwatched) require.Equal(nil, err) - res, err := meta.Accessor(&instanceUnwatched) - require.Equal(nil, err) - names := types.NamespacedName{Name: res.GetName(), Namespace: res.GetNamespace()} + names := types.NamespacedName{Name: unwatchedName, Namespace: "unwatched"} gotFinalizer := func() bool { - err := tc.k8sClient.Get(ctx, names, &instanceUnwatched) + var instance v1alpha1.StorageAccount + err := tc.k8sClient.Get(ctx, names, &instance) + require.Equal(nil, err) + res, err := meta.Accessor(&instance) require.Equal(nil, err) return HasFinalizer(res, finalizerName) } + gotNamespaceAnnotation := func() bool { + var instance v1alpha1.StorageAccount + err := tc.k8sClient.Get(ctx, names, &instance) + require.Equal(nil, err) + res, err := meta.Accessor(&instance) + require.Equal(nil, err) + return res.GetAnnotations()[namespaceAnnotation] == "azureoperator-system" + } + if configuredNamespaces == "" { // The operator should be watching all namespaces. require.Eventually( @@ -93,6 +112,13 @@ func TestTargetNamespaces(t *testing.T) { tc.retry, "instance in some namespace never got a finalizer", ) + // And there should also be a namespace annotation. + require.Eventually( + gotNamespaceAnnotation, + tc.timeoutFast, + tc.retry, + "instance in some namespace never got an operator namespace annotation", + ) } else { // We can tell that the resource isn't being reconciled if it // never gets a finalizer. @@ -102,6 +128,8 @@ func TestTargetNamespaces(t *testing.T) { time.Second, "instance in unwatched namespace got finalizer", ) + // There also shouldn't be a namespace annotation. + checkNoNamespaceAnnotation(require, &instanceUnwatched) } EnsureDelete(ctx, t, tc, &instanceDefault) @@ -109,6 +137,99 @@ func TestTargetNamespaces(t *testing.T) { EnsureDelete(ctx, t, tc, &instanceUnwatched) } +func TestOperatorNamespacePreventsReconciling(t *testing.T) { + t.Parallel() + defer PanicRecover(t) + ctx := context.Background() + + // If a resource has a different operator's namespace it won't be + // reconciled. + notMine := v1alpha1.StorageAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: "storageacct" + helpers.RandomString(6), + Namespace: "default", + Annotations: map[string]string{ + namespaceAnnotation: "hard-times", + }, + }, + Spec: v1alpha1.StorageAccountSpec{ + Kind: "BlobStorage", + Location: tc.resourceGroupLocation, + ResourceGroup: tc.resourceGroupName, + Sku: v1alpha1.StorageAccountSku{ + Name: "Standard_LRS", + }, + AccessTier: "Hot", + EnableHTTPSTrafficOnly: to.BoolPtr(true), + }, + } + + require := require.New(t) + err := tc.k8sClient.Create(ctx, ¬Mine) + require.Equal(nil, err) + defer EnsureDelete(ctx, t, tc, ¬Mine) + + names := types.NamespacedName{ + Name: notMine.ObjectMeta.Name, + Namespace: "default", + } + + gotFinalizer := func() bool { + var instance v1alpha1.StorageAccount + err := tc.k8sClient.Get(ctx, names, &instance) + require.Equal(nil, err) + res, err := meta.Accessor(&instance) + require.Equal(nil, err) + return HasFinalizer(res, finalizerName) + } + + require.Never( + gotFinalizer, + 20*time.Second, + time.Second, + "instance claimed by some other operator got finalizer", + ) + + var events corev1.EventList + err = tc.k8sClient.List(ctx, &events, &client.ListOptions{ + FieldSelector: fields.ParseSelectorOrDie("involvedObject.name=" + notMine.ObjectMeta.Name), + Namespace: "default", + }) + require.Equal(nil, err) + require.Len(events.Items, 1) + event := events.Items[0] + require.Equal(event.Type, "Warning") + require.Equal(event.Reason, "Overlap") + require.Equal(event.Message, `Operators in "azureoperator-system" and "hard-times" are both configured to manage this resource`) + + // But an instance that I've claimed gets reconciled fine. + mine := notMine + mine.ObjectMeta = metav1.ObjectMeta{ + Name: "storaceacct" + helpers.RandomString(6), + Namespace: "default", + Annotations: map[string]string{ + namespaceAnnotation: "azureoperator-system", + }, + } + EnsureInstance(ctx, t, tc, &mine) + EnsureDelete(ctx, t, tc, &mine) +} + +func checkNoNamespaceAnnotation(r *require.Assertions, instance metav1.Object) { + res, err := meta.Accessor(instance) + r.Equal(nil, err) + _, found := res.GetAnnotations()[namespaceAnnotation] + r.Equal(false, found) +} + +func checkNamespaceAnnotation(r *require.Assertions, instance metav1.Object, expected string) { + res, err := meta.Accessor(instance) + r.Equal(nil, err) + actual, found := res.GetAnnotations()[namespaceAnnotation] + r.Equal(true, found) + r.Equal(expected, actual) +} + func createNamespaces(ctx context.Context, t *testing.T, names ...string) { for _, name := range names { err := tc.k8sClient.Create(ctx, &corev1.Namespace{