-
Notifications
You must be signed in to change notification settings - Fork 5
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Allow symphonies to progress when their namespace is missing (#182)
This fixes a weird edge case in which namespaces are ripped out from under symphony resources. In this case all updates will fail - the only option is to briefly recreate the namespace, delete it, and remove any finalizers while it is deleting. Co-authored-by: Jordan Olshevski <[email protected]>
- Loading branch information
Showing
6 changed files
with
319 additions
and
4 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,130 @@ | ||
| package liveness | ||
|
|
||
| import ( | ||
| "context" | ||
| "fmt" | ||
| "time" | ||
|
|
||
| apiv1 "github.com/Azure/eno/api/v1" | ||
| "github.com/Azure/eno/internal/manager" | ||
| "github.com/go-logr/logr" | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/types" | ||
| ctrl "sigs.k8s.io/controller-runtime" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| "sigs.k8s.io/controller-runtime/pkg/handler" | ||
| "sigs.k8s.io/controller-runtime/pkg/reconcile" | ||
| ) | ||
|
|
||
| // namespaceController is responsible for progressing symphony deletion when its namespace is forcibly deleted. | ||
| // This can happen if clients get tricky with the /finalize API. | ||
| // Without this controller Eno resources will never be deleted since updates to remove the finalizers will fail. | ||
| type namespaceController struct { | ||
| client client.Client | ||
| } | ||
|
|
||
| func NewNamespaceController(mgr ctrl.Manager) error { | ||
| return ctrl.NewControllerManagedBy(mgr). | ||
| For(&corev1.Namespace{}). | ||
| Watches(&apiv1.Symphony{}, handler.EnqueueRequestsFromMapFunc(func(ctx context.Context, o client.Object) []reconcile.Request { | ||
| return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: o.GetNamespace()}}} | ||
| })). | ||
| WithLogConstructor(manager.NewLogConstructor(mgr, "namespaceController")). | ||
| Complete(&namespaceController{ | ||
| client: mgr.GetClient(), | ||
| }) | ||
| } | ||
|
|
||
| func (c *namespaceController) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { | ||
| ns := &corev1.Namespace{} | ||
| err := c.client.Get(ctx, req.NamespacedName, ns) | ||
| if client.IgnoreNotFound(err) != nil { | ||
| return ctrl.Result{}, fmt.Errorf("getting namespace: %w", err) | ||
| } | ||
|
|
||
| const annoKey = "eno.azure.io/recreation-reason" | ||
| const annoValue = "OrphanedResources" | ||
|
|
||
| // Delete the recreated namespace immediately. | ||
| // Its finalizers will keep it around until we've had time to remove our finalizers. | ||
| logger := logr.FromContextOrDiscard(ctx).WithValues("symphonyNamespace", ns.Name) | ||
| if ns.Annotations != nil && ns.Annotations[annoKey] == annoValue { | ||
| if ns.DeletionTimestamp != nil { | ||
| return ctrl.Result{}, c.cleanup(ctx, req.Name) | ||
| } | ||
| err := c.client.Delete(ctx, ns) | ||
| if err != nil { | ||
| return ctrl.Result{}, fmt.Errorf("deleting namespace: %w", err) | ||
| } | ||
| logger.V(0).Info("deleting recreated namespace") | ||
| return ctrl.Result{}, nil | ||
| } | ||
| if err == nil { // important that this is the GET error | ||
| return ctrl.Result{}, nil // namespace exists, nothing to do | ||
| } | ||
|
|
||
| // Nothing to do if the namespace doesn't have any symphonies | ||
| list := &apiv1.SymphonyList{} | ||
| err = c.client.List(ctx, list, client.InNamespace(req.Name)) | ||
| if err != nil { | ||
| return ctrl.Result{}, fmt.Errorf("listing symphonies: %w", err) | ||
| } | ||
| if len(list.Items) == 0 { | ||
| return ctrl.Result{}, nil // no orphaned resources, nothing to do | ||
| } | ||
| if time.Since(mostRecentCreation(list)) < time.Second { | ||
| return ctrl.Result{RequeueAfter: time.Second}, nil // namespace probably just hasn't hit the cache yet | ||
| } | ||
|
|
||
| // Recreate the namespace briefly so we can remove the finalizers. | ||
| // Any updates (including finalizer updates) will fail if the namespace doesn't exist. | ||
| ns.Name = req.Name | ||
| ns.Annotations = map[string]string{annoKey: annoValue} | ||
| err = c.client.Create(ctx, ns) | ||
| if err != nil { | ||
| return ctrl.Result{}, fmt.Errorf("creating namespace: %w", err) | ||
| } | ||
| logger.V(0).Info("recreating missing namespace to free orphaned symphony") | ||
| return ctrl.Result{}, nil | ||
| } | ||
|
|
||
| const removeFinalizersPatch = `[{ "op": "remove", "path": "/metadata/finalizers" }]` | ||
|
|
||
| func (c *namespaceController) cleanup(ctx context.Context, ns string) error { | ||
| logger := logr.FromContextOrDiscard(ctx).WithValues("symphonyNamespace", ns) | ||
| logger.V(0).Info("deleting any remaining symphonies in orphaned namespace") | ||
| err := c.client.DeleteAllOf(ctx, &apiv1.Symphony{}, client.InNamespace(ns)) | ||
| if err != nil { | ||
| return fmt.Errorf("deleting symphonies: %w", err) | ||
| } | ||
|
|
||
| list := &apiv1.ResourceSliceList{} | ||
| err = c.client.List(ctx, list, client.InNamespace(ns)) | ||
| if err != nil { | ||
| return fmt.Errorf("listing resource slices: %w", err) | ||
| } | ||
|
|
||
| for _, item := range list.Items { | ||
| if len(item.Finalizers) == 0 { | ||
| continue | ||
| } | ||
| err = c.client.Patch(ctx, &item, client.RawPatch(types.JSONPatchType, []byte(removeFinalizersPatch))) | ||
| if err != nil { | ||
| return fmt.Errorf("removing finalizers from resource slice %q: %w", item.Name, err) | ||
| } | ||
| logger := logger.WithValues("resourceSliceName", item.Name) | ||
| logger.V(0).Info("forcibly removed finalizers") | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func mostRecentCreation(list *apiv1.SymphonyList) time.Time { | ||
| var max time.Time | ||
| for _, item := range list.Items { | ||
| if item.CreationTimestamp.After(max) { | ||
| max = item.CreationTimestamp.Time | ||
| } | ||
| } | ||
| return max | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,86 @@ | ||
| package liveness | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| apiv1 "github.com/Azure/eno/api/v1" | ||
| "github.com/Azure/eno/internal/testutil" | ||
| "github.com/stretchr/testify/require" | ||
| corev1 "k8s.io/api/core/v1" | ||
| "k8s.io/apimachinery/pkg/api/errors" | ||
| "k8s.io/apimachinery/pkg/runtime/schema" | ||
| "k8s.io/apimachinery/pkg/runtime/serializer" | ||
| "k8s.io/client-go/rest" | ||
| "k8s.io/client-go/util/retry" | ||
| "k8s.io/kubectl/pkg/scheme" | ||
| "sigs.k8s.io/controller-runtime/pkg/client" | ||
| ) | ||
|
|
||
| func TestMissingNamespace(t *testing.T) { | ||
| ctx := testutil.NewContext(t) | ||
| mgr := testutil.NewManager(t) | ||
| cli := mgr.GetClient() | ||
|
|
||
| require.NoError(t, NewNamespaceController(mgr.Manager)) | ||
| mgr.Start(t) | ||
|
|
||
| ns := &corev1.Namespace{} | ||
| ns.Name = "test" | ||
| require.NoError(t, cli.Create(ctx, ns)) | ||
|
|
||
| sym := &apiv1.Symphony{} | ||
| sym.Name = "test-symphony" | ||
| sym.Namespace = ns.Name | ||
| sym.Finalizers = []string{"eno.azure.io/cleanup"} // this would normally be set by another controller | ||
| require.NoError(t, cli.Create(ctx, sym)) | ||
|
|
||
| // Force delete the namespace | ||
| require.NoError(t, cli.Delete(ctx, ns)) | ||
|
|
||
| conf := rest.CopyConfig(mgr.RestConfig) | ||
| conf.GroupVersion = &schema.GroupVersion{Version: "v1"} | ||
| conf.NegotiatedSerializer = serializer.WithoutConversionCodecFactory{CodecFactory: scheme.Codecs} | ||
| rc, err := rest.RESTClientFor(conf) | ||
| require.NoError(t, err) | ||
|
|
||
| err = retry.RetryOnConflict(testutil.Backoff, func() error { | ||
| cli.Get(ctx, client.ObjectKeyFromObject(ns), ns) | ||
| ns.Spec.Finalizers = nil | ||
|
|
||
| _, err = rc.Put(). | ||
| AbsPath("/api/v1/namespaces", ns.Name, "/finalize"). | ||
| Body(ns). | ||
| Do(ctx).Raw() | ||
| return err | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| // The namespace should be completely gone | ||
| testutil.Eventually(t, func() bool { | ||
| return errors.IsNotFound(cli.Get(ctx, client.ObjectKeyFromObject(ns), ns)) | ||
| }) | ||
|
|
||
| // But we should still be able to eventually remove the symphony's finalizer | ||
| require.NoError(t, cli.Delete(ctx, sym)) | ||
| testutil.Eventually(t, func() bool { | ||
| if sym.Finalizers != nil { | ||
| sym.Finalizers = nil | ||
| err = cli.Update(ctx, sym) | ||
| if err != nil { | ||
| t.Logf("error while removing finalizer from symphony: %s", err) | ||
| } | ||
| } | ||
|
|
||
| missing := errors.IsNotFound(cli.Get(ctx, client.ObjectKeyFromObject(sym), sym)) | ||
| if !missing { | ||
| t.Logf("symphony still exists") | ||
| } | ||
| return missing | ||
| }) | ||
|
|
||
| // Namespace should end up being deleted | ||
| testutil.Eventually(t, func() bool { | ||
| cli.Get(ctx, client.ObjectKeyFromObject(ns), ns) | ||
| return ns.DeletionTimestamp != nil | ||
| }) | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters