diff --git a/controllers/subnamespace_controller.go b/controllers/subnamespace_controller.go index 05de9464..f7b6a3e4 100644 --- a/controllers/subnamespace_controller.go +++ b/controllers/subnamespace_controller.go @@ -7,13 +7,13 @@ import ( accuratev2 "github.com/cybozu-go/accurate/api/accurate/v2" accuratev2ac "github.com/cybozu-go/accurate/internal/applyconfigurations/accurate/v2" - utilerrors "github.com/cybozu-go/accurate/internal/util/errors" "github.com/cybozu-go/accurate/pkg/constants" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/json" metav1ac "k8s.io/client-go/applyconfigurations/meta/v1" "k8s.io/client-go/util/csaupgrade" kstatus "sigs.k8s.io/cli-utils/pkg/kstatus/status" @@ -64,10 +64,6 @@ func (r *SubNamespaceReconciler) Reconcile(ctx context.Context, req ctrl.Request } func (r *SubNamespaceReconciler) finalize(ctx context.Context, sn *accuratev2.SubNamespace) error { - if !controllerutil.ContainsFinalizer(sn, constants.Finalizer) { - return nil - } - logger := log.FromContext(ctx) ns := &corev1.Namespace{} @@ -75,28 +71,49 @@ func (r *SubNamespaceReconciler) finalize(ctx context.Context, sn *accuratev2.Su if !apierrors.IsNotFound(err) { return err } - goto DELETE + return r.removeFinalizer(ctx, sn) } if ns.DeletionTimestamp != nil { - goto DELETE + return r.removeFinalizer(ctx, sn) } if parent := ns.Labels[constants.LabelParent]; parent != sn.Namespace { logger.Info("finalization: ignored non-child namespace", "parent", parent) - goto DELETE + return r.removeFinalizer(ctx, sn) } if err := r.Delete(ctx, ns); err != nil { - return fmt.Errorf("failed to delete namespace %s: %w", sn.Name, err) + if !apierrors.IsNotFound(err) { + return fmt.Errorf("failed to delete namespace %s: %w", ns.Name, err) + } + } else { + logger.Info("deleted namespace", "name", ns.Name) } - logger.Info("deleted namespace", "name", sn.Name) + return r.removeFinalizer(context.Background(), sn) +} -DELETE: - orig := sn.DeepCopy() +func (r *SubNamespaceReconciler) removeFinalizer(ctx context.Context, sn *accuratev2.SubNamespace) error { + if !controllerutil.ContainsFinalizer(sn, constants.Finalizer) { + return nil + } + + sn = sn.DeepCopy() controllerutil.RemoveFinalizer(sn, constants.Finalizer) - return r.Patch(ctx, sn, client.MergeFrom(orig)) + + // We'll use a JSON Merge Patch here to avoid removal of finalizers added by other controllers + patch := map[string]interface{}{ + "metadata": map[string]interface{}{ + "finalizers": sn.GetFinalizers(), + }, + } + patchBytes, err := json.Marshal(patch) + if err != nil { + return err + } + + return client.IgnoreNotFound(r.Patch(ctx, sn, client.RawPatch(types.MergePatchType, patchBytes))) } func (r *SubNamespaceReconciler) reconcileNS(ctx context.Context, sn *accuratev2.SubNamespace) error { @@ -115,7 +132,7 @@ func (r *SubNamespaceReconciler) reconcileNS(ctx context.Context, sn *accuratev2 constants.LabelParent: sn.Namespace, } if err := r.Create(ctx, ns); err != nil { - return utilerrors.Ignore(err, utilerrors.IsNamespaceTerminating) + return fmt.Errorf("failed to create namespace %s: %w", ns.Name, err) } logger.Info("created a sub namespace", "name", sn.Name) } @@ -162,11 +179,10 @@ func (r *SubNamespaceReconciler) SetupWithManager(mgr ctrl.Manager) error { Namespace: parent, Name: o.GetName(), }}) - return } if o.GetDeletionTimestamp() != nil { - // The namespace has no parent and is in terminating state. - // Let's find all (conflicting) subnamespaces that might want to recreate it. + // The namespace is in terminating state. + // Let's find all subnamespaces that might want to recreate it. snList := &accuratev2.SubNamespaceList{} err := r.List(ctx, snList, client.MatchingFields{constants.SubNamespaceNameKey: o.GetName()}) if err != nil { diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index c8cc0db5..416b3704 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -340,6 +340,59 @@ var _ = Describe("kubectl accurate", func() { Expect(sn.Labels).To(HaveKeyWithValue(constants.LabelParent, "conflict-root1")) }) + It("should (re)create sub-namespace when conflicting sub-namespace deleted", func() { + By("preparing namespaces") + kubectlSafe(nil, "create", "ns", "sub-conflict-root1") + kubectlSafe(nil, "accurate", "ns", "set-type", "sub-conflict-root1", "root") + kubectlSafe(nil, "create", "ns", "sub-conflict-root2") + kubectlSafe(nil, "accurate", "ns", "set-type", "sub-conflict-root2", "root") + + By("creating subnamespace") + kubectlSafe(nil, "accurate", "sub", "create", "sub-conflict-sn1", "sub-conflict-root1") + subNsLabelsFn := func() (map[string]string, error) { + out, err := kubectl(nil, "get", "ns", "sub-conflict-sn1", "-o", "json") + if err != nil { + return nil, err + } + ns := &corev1.Namespace{} + if err := json.Unmarshal(out, ns); err != nil { + return nil, err + } + return ns.Labels, nil + } + Eventually(subNsLabelsFn).Should(HaveKeyWithValue(constants.LabelParent, "sub-conflict-root1")) + + By("creating conflicting subnamespace") + // Cannot use "kubectl accurate" here, since conflict is validated client-side. + kubectlSafe([]byte(` +apiVersion: accurate.cybozu.com/v2 +kind: SubNamespace +metadata: + name: sub-conflict-sn1 + namespace: sub-conflict-root2 +`), "apply", "-f", "-") + var conditions []metav1.Condition + sn1ConditionsFn := func() ([]metav1.Condition, error) { + out, err := kubectl(nil, "get", "-n", "sub-conflict-root2", "subnamespaces", "sub-conflict-sn1", "-o", "json") + if err != nil { + return nil, err + } + sn := &accuratev2.SubNamespace{} + if err := json.Unmarshal(out, sn); err != nil { + return nil, err + } + conditions = sn.Status.Conditions + return conditions, nil + } + Eventually(sn1ConditionsFn).Should(HaveLen(1)) + Expect(conditions[0].Reason).To(Equal("Conflict")) + + By("deleting subnamespace") + kubectlSafe(nil, "accurate", "sub", "delete", "sub-conflict-sn1") + Eventually(sn1ConditionsFn).Should(BeEmpty()) + Eventually(subNsLabelsFn).Should(HaveKeyWithValue(constants.LabelParent, "sub-conflict-root2")) + }) + It("should propagate ServiceAccount w/o secrets field", func() { // From Kubernetes 1.24, the auto-generation of secret-based service account tokens has been disabled by default. // So the secrets field in the ServiceAccount is not updated. But when upgrading Kubernetes from 1.23 or lower,