diff --git a/internal/controllers/reconciliation/controller.go b/internal/controllers/reconciliation/controller.go index b38ae258..eb77f3b2 100644 --- a/internal/controllers/reconciliation/controller.go +++ b/internal/controllers/reconciliation/controller.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "os" + "strings" "time" "k8s.io/apimachinery/pkg/api/meta" @@ -150,7 +151,7 @@ func (c *Controller) Reconcile(ctx context.Context, req *reconstitution.Request) // Fetch the current resource current, hasChanged, err := c.getCurrent(ctx, resource) - if client.IgnoreNotFound(err) != nil { + if client.IgnoreNotFound(err) != nil && !isErrMissingNS(err) { return ctrl.Result{}, fmt.Errorf("getting current state: %w", err) } @@ -405,3 +406,14 @@ func patchResourceState(deleted bool, ready *metav1.Time) flowcontrol.StatusPatc } } } + +// isErrMissingNS returns true when given the client-go error returned by mutating requests that do not include a namespace. +// Sadly, this error isn't exposed anywhere - it's just a plain string, so we have to do string matching here. +// +// https://github.com/kubernetes/kubernetes/blob/9edabd617945cd23111fd46cfc9a09fe37ed194a/staging/src/k8s.io/client-go/rest/request.go#L1048 +func isErrMissingNS(err error) bool { + if err == nil { + return false + } + return strings.Contains(err.Error(), "an empty namespace may not be set") +} diff --git a/internal/controllers/reconciliation/edgecase_test.go b/internal/controllers/reconciliation/edgecase_test.go new file mode 100644 index 00000000..cd0feb5c --- /dev/null +++ b/internal/controllers/reconciliation/edgecase_test.go @@ -0,0 +1,107 @@ +package reconciliation + +import ( + "context" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/client-go/util/retry" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + + apiv1 "github.com/Azure/eno/api/v1" + "github.com/Azure/eno/internal/testutil" + krmv1 "github.com/Azure/eno/pkg/krm/functions/api/v1" +) + +// TestMissingNamespace proves that resynthesis is not blocked by resources that lack a namespace. +func TestMissingNamespace(t *testing.T) { + ctx := testutil.NewContext(t) + mgr := testutil.NewManager(t) + upstream := mgr.GetClient() + + registerControllers(t, mgr) + namespace := atomic.Pointer[string]{} + testutil.WithFakeExecutor(t, mgr, func(ctx context.Context, s *apiv1.Synthesizer, input *krmv1.ResourceList) (*krmv1.ResourceList, error) { + output := &krmv1.ResourceList{} + output.Items = []*unstructured.Unstructured{{ + Object: map[string]any{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{ + "name": "test-obj", + "namespace": namespace.Load(), // bad news + }, + }, + }} + return output, nil + }) + + // Test subject + setupTestSubject(t, mgr) + mgr.Start(t) + syn, comp := writeGenericComposition(t, upstream) + + // Initial synthesis + testutil.Eventually(t, func() bool { + err := upstream.Get(ctx, client.ObjectKeyFromObject(comp), comp) + return err == nil && comp.Status.CurrentSynthesis != nil && comp.Status.CurrentSynthesis.Synthesized != nil && comp.Status.CurrentSynthesis.ObservedSynthesizerGeneration == syn.Generation + }) + + // Fixing the namespace should be possible + namespace.Store(ptr.To("default")) + err := retry.RetryOnConflict(testutil.Backoff, func() error { + upstream.Get(ctx, client.ObjectKeyFromObject(syn), syn) + syn.Spec.Image = "updated" + return upstream.Update(ctx, syn) + }) + require.NoError(t, err) + + testutil.Eventually(t, func() bool { + err := upstream.Get(ctx, client.ObjectKeyFromObject(comp), comp) + return err == nil && comp.Status.CurrentSynthesis != nil && comp.Status.CurrentSynthesis.Reconciled != nil && comp.Status.CurrentSynthesis.ObservedSynthesizerGeneration == syn.Generation + }) +} + +// TestMissingNamespaceDeletion proves that composition deletion is not blocked by resources that lack a namespace. +func TestMissingNamespaceDeletion(t *testing.T) { + ctx := testutil.NewContext(t) + mgr := testutil.NewManager(t) + upstream := mgr.GetClient() + + registerControllers(t, mgr) + testutil.WithFakeExecutor(t, mgr, func(ctx context.Context, s *apiv1.Synthesizer, input *krmv1.ResourceList) (*krmv1.ResourceList, error) { + output := &krmv1.ResourceList{} + output.Items = []*unstructured.Unstructured{{ + Object: map[string]any{ + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": map[string]any{ + "name": "test-obj", + "namespace": "", // bad news + }, + }, + }} + return output, nil + }) + + // Test subject + setupTestSubject(t, mgr) + mgr.Start(t) + syn, comp := writeGenericComposition(t, upstream) + + // Initial synthesis + testutil.Eventually(t, func() bool { + err := upstream.Get(ctx, client.ObjectKeyFromObject(comp), comp) + return err == nil && comp.Status.CurrentSynthesis != nil && comp.Status.CurrentSynthesis.Synthesized != nil && comp.Status.CurrentSynthesis.ObservedSynthesizerGeneration == syn.Generation + }) + + // Deleting the composition should succeed eventually + require.NoError(t, upstream.Delete(ctx, comp)) + testutil.Eventually(t, func() bool { + return errors.IsNotFound(upstream.Get(ctx, client.ObjectKeyFromObject(comp), comp)) + }) +}