From 44066ac0267fa4bc05d94c8aab64acf34125df6d Mon Sep 17 00:00:00 2001 From: David Bond Date: Wed, 4 Nov 2026 18:56:58 +0000 Subject: [PATCH] cmd/k8s-operator: keep ProxyGroup state Secrets until cleanup finishes (#22155) This commit removes the owner reference from ProxyGroup state Secrets, also from Secrets that an older operator created. On a Foreground delete, the garbage collector deleted these Secrets before the finalizers ran. The finalizers read them to find devices, so cleanup failed and devices stayed in the tailnet. The ProxyGroup cleanup now deletes the Secrets after the devices, and the reconciler watches them by label. The kube-apiserver cleanup finds its cert resources by label, so it does not need the state Secrets. A new e2e test covers Foreground deletes. Fixes #22072 Signed-off-by: David Bond --- cmd/k8s-operator/api-server-proxy-pg.go | 4 +- cmd/k8s-operator/api-server-proxy-pg_test.go | 6 +- cmd/k8s-operator/e2e/main_test.go | 5 +- cmd/k8s-operator/e2e/proxygroup_test.go | 127 +++++++++++++++++++ cmd/k8s-operator/ingress-for-pg.go | 24 ++++ cmd/k8s-operator/operator.go | 18 +++ cmd/k8s-operator/operator_test.go | 35 +++++ cmd/k8s-operator/proxygroup.go | 38 +++--- cmd/k8s-operator/proxygroup_specs.go | 11 +- cmd/k8s-operator/proxygroup_test.go | 117 ++++++++++++++++- 10 files changed, 356 insertions(+), 29 deletions(-) diff --git a/cmd/k8s-operator/api-server-proxy-pg.go b/cmd/k8s-operator/api-server-proxy-pg.go index 37260c7a0..08bb405b9 100644 --- a/cmd/k8s-operator/api-server-proxy-pg.go +++ b/cmd/k8s-operator/api-server-proxy-pg.go @@ -253,7 +253,9 @@ func (r *KubeAPIServerTSServiceReconciler) maybeCleanup(ctx context.Context, ser return fmt.Errorf("error deleting Tailscale Service: %w", err) } - if err = cleanupCertResources(ctx, r.Client, r.tsNamespace, serviceName, pg); err != nil { + // Find the cert resources by label, because the state Secrets that hold + // the DNS name can already be gone. + if err = cleanupAllCertResources(ctx, r.Client, r.tsNamespace, pg.Name); err != nil { return fmt.Errorf("failed to clean up cert resources: %w", err) } diff --git a/cmd/k8s-operator/api-server-proxy-pg_test.go b/cmd/k8s-operator/api-server-proxy-pg_test.go index 889ef064b..b7c35f437 100644 --- a/cmd/k8s-operator/api-server-proxy-pg_test.go +++ b/cmd/k8s-operator/api-server-proxy-pg_test.go @@ -264,7 +264,11 @@ func TestAPIServerProxyReconciler(t *testing.T) { tsoperator.SetProxyGroupCondition(pg, tsapi.KubeAPIServerProxyConfigured, metav1.ConditionTrue, reasonKubeAPIServerProxyConfigured, "", 1, r.clock, r.logger) pg.Status.URL = "https://" + updatedDomain - // Delete the ProxyGroup and verify Tailscale Service and cert resources are cleaned up. + // Delete the ProxyGroup and verify Tailscale Service and cert resources are + // cleaned up. Delete the state Secret first, as a Foreground delete does + // on older versions of the operator, so the cleanup cannot read the DNS + // name from it. + mustDeleteAll(t, fc, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "test-pg-0", Namespace: ns}}) if err := fc.Delete(t.Context(), pg); err != nil { t.Fatalf("deleting ProxyGroup: %v", err) } diff --git a/cmd/k8s-operator/e2e/main_test.go b/cmd/k8s-operator/e2e/main_test.go index f7ec78b04..4e9d99a63 100644 --- a/cmd/k8s-operator/e2e/main_test.go +++ b/cmd/k8s-operator/e2e/main_test.go @@ -53,8 +53,9 @@ func createAndCleanup(t *testing.T, cl client.Client, obj client.Object) { t.Cleanup(func() { // Use context.Background() for cleanup, as t.Context() is cancelled - // just before cleanup functions are called. - if err := cl.Delete(context.Background(), obj); err != nil { + // just before cleanup functions are called. The test may have + // deleted the object itself. + if err := client.IgnoreNotFound(cl.Delete(context.Background(), obj)); err != nil { t.Errorf("error cleaning up %s %s/%s: %s", obj.GetObjectKind().GroupVersionKind(), obj.GetNamespace(), obj.GetName(), err) } }) diff --git a/cmd/k8s-operator/e2e/proxygroup_test.go b/cmd/k8s-operator/e2e/proxygroup_test.go index 9f19eacbd..79ea5ae10 100644 --- a/cmd/k8s-operator/e2e/proxygroup_test.go +++ b/cmd/k8s-operator/e2e/proxygroup_test.go @@ -8,9 +8,12 @@ import ( "testing" "time" + corev1 "k8s.io/api/core/v1" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "tailscale.com/client/tailscale/v2" tsoperator "tailscale.com/k8s-operator" tsapi "tailscale.com/k8s-operator/apis/v1alpha1" "tailscale.com/tstest" @@ -54,6 +57,130 @@ func TestProxyGroupStaticEndpoints(t *testing.T) { waitForNoPortEnv(t, "proxygroup", pg.Name) } +// TestProxyGroupForegroundDelete checks that a ProxyGroup deleted with a +// Foreground cascade finishes its cleanup: the ProxyGroup goes away, its +// devices are deleted from the tailnet, and its state Secrets are deleted. A +// Foreground delete makes the garbage collector remove owned objects before +// the owner, so the state Secrets must not be owned by the ProxyGroup. +// +// See [TestMain] for test requirements. +func TestProxyGroupForegroundDelete(t *testing.T) { + if tnClient == nil { + t.Skip("TestProxyGroupForegroundDelete requires a working tailnet client") + } + t.Parallel() + + t.Run("egress", func(t *testing.T) { + t.Parallel() + + pg := &tsapi.ProxyGroup{ + ObjectMeta: metav1.ObjectMeta{Name: generateName("fg-delete-egress")}, + Spec: tsapi.ProxyGroupSpec{ + Type: tsapi.ProxyGroupTypeEgress, + Replicas: new(int32(2)), + }, + } + createAndCleanup(t, kubeClient, pg) + waitForProxyGroupDevices(t, pg.Name, 2, false) + + foregroundDeleteProxyGroup(t, pg) + }) + + t.Run("kube-apiserver", func(t *testing.T) { + t.Parallel() + + pg := &tsapi.ProxyGroup{ + ObjectMeta: metav1.ObjectMeta{Name: generateName("fg-delete-api")}, + Spec: tsapi.ProxyGroupSpec{ + Type: tsapi.ProxyGroupTypeKubernetesAPIServer, + ProxyClass: applyProxyGroupProxyClass(t), + Replicas: new(int32(2)), + KubeAPIServer: &tsapi.KubeAPIServerConfig{ + Mode: new(tsapi.APIServerProxyModeNoAuth), + }, + }, + } + createAndCleanup(t, kubeClient, pg) + waitForKubeAPIServerProxyReady(t, pg.Name) + + foregroundDeleteProxyGroup(t, pg) + waitForKubeAPIServerProxyShutdown(t, pg.Name) + }) +} + +// foregroundDeleteProxyGroup deletes the ProxyGroup with a Foreground cascade +// and waits for it to go away. It then checks that the ProxyGroup's devices +// are deleted from the tailnet and that its state Secrets are deleted. +func foregroundDeleteProxyGroup(t *testing.T, pg *tsapi.ProxyGroup) { + t.Helper() + + replicas := int(*pg.Spec.Replicas) + + deviceIDs := make(map[string]bool) + for _, id := range deviceIDsByTailnetIP(t, "proxygroup", pg.Name) { + deviceIDs[id] = true + } + if len(deviceIDs) != replicas { + t.Fatalf("ProxyGroup %s has device IDs %v in its state Secrets, want %d devices", pg.Name, deviceIDs, replicas) + } + + // Whether the garbage collector deletes owned state Secrets before the + // operator's finalizers run depends on timing, so also check for the + // owner reference directly. + var secrets corev1.SecretList + if err := kubeClient.List(t.Context(), &secrets, client.InNamespace("tailscale"), stateSecretLabels(pg.Name)); err != nil { + t.Fatalf("listing state Secrets: %v", err) + } + for _, s := range secrets.Items { + for _, ref := range s.OwnerReferences { + if ref.UID == pg.UID { + t.Fatalf("state Secret %s is owned by ProxyGroup %s, so a Foreground delete can remove it before the finalizers run", s.Name, pg.Name) + } + } + } + + if err := kubeClient.Delete(t.Context(), pg, client.PropagationPolicy(metav1.DeletePropagationForeground)); err != nil { + t.Fatalf("deleting ProxyGroup %s: %v", pg.Name, err) + } + + if err := tstest.WaitFor(3*time.Minute, func() error { + cur := &tsapi.ProxyGroup{ObjectMeta: metav1.ObjectMeta{Name: pg.Name}} + if err := get(t.Context(), kubeClient, cur); !apierrors.IsNotFound(err) { + return fmt.Errorf("ProxyGroup %s still exists with finalizers %v (err=%v)", pg.Name, cur.Finalizers, err) + } + return nil + }); err != nil { + t.Fatalf("waiting for ProxyGroup %s to be deleted: %v", pg.Name, err) + } + + for id := range deviceIDs { + _, err := tsClient.Devices().Get(t.Context(), id) + switch { + case tailscale.IsNotFound(err): + case err != nil: + t.Errorf("getting device %s: %v", id, err) + default: + t.Errorf("device %s of ProxyGroup %s is still in the tailnet", id, pg.Name) + } + } + + if err := kubeClient.List(t.Context(), &secrets, client.InNamespace("tailscale"), stateSecretLabels(pg.Name)); err != nil { + t.Fatalf("listing state Secrets: %v", err) + } + for _, s := range secrets.Items { + t.Errorf("state Secret %s of ProxyGroup %s still exists", s.Name, pg.Name) + } +} + +// stateSecretLabels matches the state Secrets of the ProxyGroup. +func stateSecretLabels(pgName string) client.MatchingLabels { + return client.MatchingLabels{ + "tailscale.com/parent-resource-type": "proxygroup", + "tailscale.com/parent-resource": pgName, + "tailscale.com/secret-type": "state", + } +} + // waitForProxyGroupDevices waits for the ProxyGroup to be ready with the given // number of devices in its status, and for every device to report static // endpoints if wantStaticEndpoints is true, or none if it is false. diff --git a/cmd/k8s-operator/ingress-for-pg.go b/cmd/k8s-operator/ingress-for-pg.go index f76a7da2a..9b8c70380 100644 --- a/cmd/k8s-operator/ingress-for-pg.go +++ b/cmd/k8s-operator/ingress-for-pg.go @@ -1014,6 +1014,30 @@ func cleanupCertResources(ctx context.Context, cl client.Client, tsNamespace str return nil } +// cleanupAllCertResources deletes the TLS Secrets and associated RBAC +// resources for every domain of the ProxyGroup. It finds them by label, so it +// does not need the ProxyGroup's state Secrets to work out the domain names. +func cleanupAllCertResources(ctx context.Context, cl client.Client, tsNamespace, pgName string) error { + opts := []client.DeleteAllOfOption{ + client.InNamespace(tsNamespace), + client.MatchingLabels{ + kubetypes.LabelManaged: "true", + labelProxyGroup: pgName, + }, + client.HasLabels{labelDomain}, + } + if err := cl.DeleteAllOf(ctx, &rbacv1.RoleBinding{}, opts...); err != nil { + return fmt.Errorf("error deleting cert RoleBindings for ProxyGroup %s: %w", pgName, err) + } + if err := cl.DeleteAllOf(ctx, &rbacv1.Role{}, opts...); err != nil { + return fmt.Errorf("error deleting cert Roles for ProxyGroup %s: %w", pgName, err) + } + if err := cl.DeleteAllOf(ctx, &corev1.Secret{}, opts...); err != nil { + return fmt.Errorf("error deleting cert Secrets for ProxyGroup %s: %w", pgName, err) + } + return nil +} + // requeueInterval returns a time duration between 5 and 10 minutes, which is // the period of time after which an HA Ingress, whose Tailscale Service has been newly // created or changed, needs to be requeued. This is to protect against diff --git a/cmd/k8s-operator/operator.go b/cmd/k8s-operator/operator.go index 2619d322f..430ac3015 100644 --- a/cmd/k8s-operator/operator.go +++ b/cmd/k8s-operator/operator.go @@ -728,6 +728,7 @@ func runReconcilers(opts reconcilerOpts) { nodeFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(nodeHandlerForProxyGroup(mgr.GetClient(), opts.defaultProxyClass, startlog)) saFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(serviceAccountHandlerForProxyGroup(mgr.GetClient(), startlog)) acmeSecretFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(acmeAccountsSecretHandlerForProxyGroup(mgr.GetClient(), opts.tailscaleNamespace, opts.sharedACMEAccountKey, startlog)) + stateSecretFilterForProxyGroup := handler.EnqueueRequestsFromMapFunc(stateSecretHandlerForProxyGroup) err = builder.ControllerManagedBy(mgr). For(&tsapi.ProxyGroup{}). Named("proxygroup-reconciler"). @@ -739,6 +740,8 @@ func runReconcilers(opts reconcilerOpts) { // The shared ACME accounts Secret has no ProxyGroup owner ref, so // watch it by name to react to its deletion/recreation. Watches(&corev1.Secret{}, acmeSecretFilterForProxyGroup). + // State Secrets have no owner ref, so watch them by label. + Watches(&corev1.Secret{}, stateSecretFilterForProxyGroup). Watches(&rbacv1.Role{}, ownedByProxyGroupFilter). Watches(&rbacv1.RoleBinding{}, ownedByProxyGroupFilter). Watches(&tsapi.ProxyClass{}, proxyClassFilterForProxyGroup). @@ -1209,6 +1212,21 @@ func acmeAccountsSecretHandlerForProxyGroup(cl client.Client, tsNamespace string } } +// stateSecretHandlerForProxyGroup enqueues the ProxyGroup that a state Secret +// belongs to. State Secrets carry no owner reference, so the owner-based Secret +// watch never matches them. +func stateSecretHandlerForProxyGroup(_ context.Context, o client.Object) []reconcile.Request { + secret, ok := o.(*corev1.Secret) + if !ok || !isPGStateSecret(secret) { + return nil + } + pgName := secret.ObjectMeta.Labels[LabelParentName] + if pgName == "" { + return nil + } + return []reconcile.Request{{NamespacedName: types.NamespacedName{Name: pgName}}} +} + // serviceHandlerForIngress returns a handler for Service events for ingress // reconciler that ensures that if the Service associated with an event is of // interest to the reconciler, the associated Ingress(es) gets be reconciled. diff --git a/cmd/k8s-operator/operator_test.go b/cmd/k8s-operator/operator_test.go index a8e985547..8646bac25 100644 --- a/cmd/k8s-operator/operator_test.go +++ b/cmd/k8s-operator/operator_test.go @@ -2243,3 +2243,38 @@ func TestNodeHandlerForConnector(t *testing.T) { } }) } + +func TestStateSecretHandlerForProxyGroup(t *testing.T) { + for _, tc := range []struct { + name string + labels map[string]string + want []reconcile.Request + }{ + { + name: "state_secret", + labels: pgSecretLabels("test", kubetypes.LabelSecretTypeState), + want: []reconcile.Request{{NamespacedName: types.NamespacedName{Name: "test"}}}, + }, + { + name: "config_secret", + labels: pgSecretLabels("test", kubetypes.LabelSecretTypeConfig), + }, + { + name: "unmanaged_secret", + }, + } { + t.Run(tc.name, func(t *testing.T) { + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-0", + Namespace: tsNamespace, + Labels: tc.labels, + }, + } + got := stateSecretHandlerForProxyGroup(t.Context(), secret) + if diff := cmp.Diff(got, tc.want); diff != "" { + t.Fatalf("unexpected reconcile requests (-got +want):\n%s", diff) + } + }) + } +} diff --git a/cmd/k8s-operator/proxygroup.go b/cmd/k8s-operator/proxygroup.go index 78c3bf952..b8455e4b2 100644 --- a/cmd/k8s-operator/proxygroup.go +++ b/cmd/k8s-operator/proxygroup.go @@ -189,6 +189,21 @@ func (r *ProxyGroupReconciler) reconcilePG(ctx context.Context, tsClient tsclien } } + // State Secrets are precreated so that the operator can find and delete + // them when the ProxyGroup is deleted. Do this before anything that can + // block provisioning, so that the update removes the owner reference that + // older versions of the operator set. + stateSecrets := pgStateSecrets(pg, r.tsNamespace) + for _, sec := range stateSecrets { + if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, sec, func(s *corev1.Secret) { + s.ObjectMeta.Labels = sec.ObjectMeta.Labels + s.ObjectMeta.Annotations = sec.ObjectMeta.Annotations + s.ObjectMeta.OwnerReferences = sec.ObjectMeta.OwnerReferences + }); err != nil { + return r.notReadyErrf(pg, logger, "error provisioning state Secrets: %w", err) + } + } + proxyClassName := r.defaultProxyClass if pg.Spec.ProxyClass != "" { proxyClassName = pg.Spec.ProxyClass @@ -336,18 +351,6 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, tsClient tscl return r.notReadyErrf(pg, logger, "error provisioning config Secrets: %w", err) } - // State secrets are precreated so we can use the ProxyGroup CR as their owner ref. - stateSecrets := pgStateSecrets(pg, r.tsNamespace) - for _, sec := range stateSecrets { - if _, err := createOrUpdate(ctx, r.Client, r.tsNamespace, sec, func(s *corev1.Secret) { - s.ObjectMeta.Labels = sec.ObjectMeta.Labels - s.ObjectMeta.Annotations = sec.ObjectMeta.Annotations - s.ObjectMeta.OwnerReferences = sec.ObjectMeta.OwnerReferences - }); err != nil { - return r.notReadyErrf(pg, logger, "error provisioning state Secrets: %w", err) - } - } - // auth mode kube-apiserver ProxyGroups use a statically created // ServiceAccount to keep ClusterRole creation permissions limited to the // helm chart installer. @@ -604,9 +607,10 @@ func (r *ProxyGroupReconciler) cleanupDanglingResources(ctx context.Context, tsC return nil } -// maybeCleanup just deletes the device from the tailnet. All the kubernetes -// resources linked to a ProxyGroup will get cleaned up via owner references -// (which we can use because they are all in the same namespace). +// maybeCleanup deletes the ProxyGroup's devices from the tailnet, then deletes +// its state Secrets (see [pgStateSecrets]). The other Kubernetes resources +// linked to a ProxyGroup get cleaned up via owner references (which we can use +// because they are all in the same namespace). func (r *ProxyGroupReconciler) maybeCleanup(ctx context.Context, tsClient tsclient.Client, pg *tsapi.ProxyGroup) (bool, error) { logger := r.logger(pg.Name) @@ -634,6 +638,10 @@ func (r *ProxyGroupReconciler) maybeCleanup(ctx context.Context, tsClient tsclie return false, fmt.Errorf("error cleaning up metrics resources: %w", err) } + if err := r.DeleteAllOf(ctx, &corev1.Secret{}, client.InNamespace(r.tsNamespace), client.MatchingLabels(pgSecretLabels(pg.Name, kubetypes.LabelSecretTypeState))); err != nil { + return false, fmt.Errorf("error deleting state Secrets: %w", err) + } + logger.Infof("cleaned up ProxyGroup resources") r.mu.Lock() r.ensureStateRemovedForProxyGroup(pg) diff --git a/cmd/k8s-operator/proxygroup_specs.go b/cmd/k8s-operator/proxygroup_specs.go index f317c0a8d..79e9a2b61 100644 --- a/cmd/k8s-operator/proxygroup_specs.go +++ b/cmd/k8s-operator/proxygroup_specs.go @@ -560,14 +560,17 @@ func isAuthAPIServerProxy(pg *tsapi.ProxyGroup) bool { *pg.Spec.KubeAPIServer.Mode == tsapi.APIServerProxyModeAuth } +// pgStateSecrets returns the state Secrets for each replica of the ProxyGroup. +// The state Secrets have no owner reference. The ProxyGroup finalizer reads +// them to find the tailnet devices, so a foreground delete must not remove +// them first. ProxyGroupReconciler.maybeCleanup deletes them. func pgStateSecrets(pg *tsapi.ProxyGroup, namespace string) (secrets []*corev1.Secret) { for i := range pgReplicas(pg) { secrets = append(secrets, &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: pgStateSecretName(pg.Name, i), - Namespace: namespace, - Labels: pgSecretLabels(pg.Name, kubetypes.LabelSecretTypeState), - OwnerReferences: pgOwnerReference(pg), + Name: pgStateSecretName(pg.Name, i), + Namespace: namespace, + Labels: pgSecretLabels(pg.Name, kubetypes.LabelSecretTypeState), }, }) } diff --git a/cmd/k8s-operator/proxygroup_test.go b/cmd/k8s-operator/proxygroup_test.go index d96381250..986b6ad5c 100644 --- a/cmd/k8s-operator/proxygroup_test.go +++ b/cmd/k8s-operator/proxygroup_test.go @@ -1109,6 +1109,111 @@ func TestProxyGroup(t *testing.T) { }) } +// TestProxyGroupDeleteStateSecrets checks that the state Secrets have no owner +// reference, also when an older operator created them, and that the ProxyGroup +// cleanup deletes them. +func TestProxyGroupDeleteStateSecrets(t *testing.T) { + for _, tc := range []struct { + name string + pgType tsapi.ProxyGroupType + proxyClass string + }{ + { + name: "egress", + pgType: tsapi.ProxyGroupTypeEgress, + }, + { + name: "kube_apiserver", + pgType: tsapi.ProxyGroupTypeKubernetesAPIServer, + }, + { + // The reconcile stops at the missing ProxyClass, but it + // must still remove the old owner reference. + name: "proxyclass_missing", + pgType: tsapi.ProxyGroupTypeEgress, + proxyClass: "missing", + }, + } { + t.Run(tc.name, func(t *testing.T) { + pg := &tsapi.ProxyGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + UID: "test-uid", + Finalizers: []string{FinalizerName}, + }, + Spec: tsapi.ProxyGroupSpec{ + Type: tc.pgType, + Replicas: new(int32(2)), + ProxyClass: tc.proxyClass, + }, + } + if tc.pgType == tsapi.ProxyGroupTypeKubernetesAPIServer { + pg.Spec.KubeAPIServer = &tsapi.KubeAPIServerConfig{ + Mode: new(tsapi.APIServerProxyModeNoAuth), + } + } + // A state Secret created by an older version of the operator, + // with an owner reference. + oldStateSecret := pgStateSecrets(pg, tsNamespace)[0] + oldStateSecret.OwnerReferences = pgOwnerReference(pg) + + fc := fake.NewClientBuilder(). + WithScheme(tsapi.GlobalScheme). + WithObjects(pg, oldStateSecret). + WithStatusSubresource(pg). + Build() + tsClient := &fakeTSClient{} + zl, _ := zap.NewDevelopment() + reconciler := &ProxyGroupReconciler{ + tsNamespace: tsNamespace, + tsProxyImage: testProxyImage, + k8sProxyImage: "tailscale/k8s-proxy:test", + defaultTags: []string{"tag:test-tag"}, + tsFirewallMode: "auto", + + Client: fc, + clients: tsclient.NewProvider(tsClient), + recorder: record.NewFakeRecorder(10), + log: zl.Sugar(), + clock: tstest.NewClock(tstest.ClockOpts{}), + reissuer: tailscaled.NewReissuer(), + } + + expectReconciled(t, reconciler, "", pg.Name) + var secrets corev1.SecretList + if err := fc.List(t.Context(), &secrets, client.InNamespace(tsNamespace), client.MatchingLabels(pgSecretLabels(pg.Name, kubetypes.LabelSecretTypeState))); err != nil { + t.Fatal(err) + } + if len(secrets.Items) != 2 { + t.Fatalf("got %d state Secrets, want 2", len(secrets.Items)) + } + for _, s := range secrets.Items { + if len(s.OwnerReferences) != 0 { + t.Errorf("state Secret %s has owner references %v, want none", s.Name, s.OwnerReferences) + } + } + + addNodeIDToStateSecrets(t, fc, pg) + if err := fc.Delete(t.Context(), pg); err != nil { + t.Fatal(err) + } + + expectReconciled(t, reconciler, "", pg.Name) + expectMissing[tsapi.ProxyGroup](t, fc, "", pg.Name) + if diff := cmp.Diff(tsClient.deleted, []string{"nodeid-0", "nodeid-1"}); diff != "" { + t.Fatalf("unexpected deleted devices (-got +want):\n%s", diff) + } + // The config Secrets have an owner reference, and the fake + // client does not garbage collect them. + var wantSecrets []string + if tc.proxyClass == "" { + wantSecrets = []string{"test-0-config", "test-1-config"} + } + expectSecrets(t, fc, wantSecrets) + }) + } +} + func TestProxyGroupTypes(t *testing.T) { pc := &tsapi.ProxyClass{ ObjectMeta: metav1.ObjectMeta{ @@ -2135,13 +2240,13 @@ func expectProxyGroupResources(t *testing.T, fc client.WithWatch, pg *tsapi.Prox expectMissing[appsv1.StatefulSet](t, fc, statefulSet.Namespace, statefulSet.Name) } + // The state Secrets are created before the ProxyClass is checked, so + // they exist even when the other resources do not. var expectedSecrets []string - if shouldExist { - for i := range pgReplicas(pg) { - expectedSecrets = append(expectedSecrets, - fmt.Sprintf("%s-%d", pg.Name, i), - pgConfigSecretName(pg.Name, i), - ) + for i := range pgReplicas(pg) { + expectedSecrets = append(expectedSecrets, fmt.Sprintf("%s-%d", pg.Name, i)) + if shouldExist { + expectedSecrets = append(expectedSecrets, pgConfigSecretName(pg.Name, i)) } } expectSecrets(t, fc, expectedSecrets)