diff --git a/cmd/k8s-operator/connector_test.go b/cmd/k8s-operator/connector_test.go index f32fe3282..d5829c37f 100644 --- a/cmd/k8s-operator/connector_test.go +++ b/cmd/k8s-operator/connector_test.go @@ -80,7 +80,7 @@ func TestConnector(t *testing.T) { app: kubetypes.AppConnector, } expectEqual(t, fc, expectedSecret(t, fc, opts)) - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // Connector status should get updated with the IP/hostname info when available. const hostname = "foo.tailnetxyz.ts.net" @@ -106,7 +106,7 @@ func TestConnector(t *testing.T) { opts.subnetRoutes = "10.40.0.0/14,10.44.0.0/20" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // Remove a route. mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) { @@ -114,7 +114,7 @@ func TestConnector(t *testing.T) { }) opts.subnetRoutes = "10.44.0.0/20" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // Remove the subnet router. mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) { @@ -122,7 +122,7 @@ func TestConnector(t *testing.T) { }) opts.subnetRoutes = "" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // Re-add the subnet router. mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) { @@ -132,7 +132,7 @@ func TestConnector(t *testing.T) { }) opts.subnetRoutes = "10.44.0.0/20" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // Delete the Connector. if err = fc.Delete(context.Background(), cn); err != nil { @@ -176,7 +176,7 @@ func TestConnector(t *testing.T) { app: kubetypes.AppConnector, } expectEqual(t, fc, expectedSecret(t, fc, opts)) - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // Add an exit node. mustUpdate[tsapi.Connector](t, fc, "", "test", func(conn *tsapi.Connector) { @@ -184,7 +184,7 @@ func TestConnector(t *testing.T) { }) opts.isExitNode = true expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // Delete the Connector. if err = fc.Delete(context.Background(), cn); err != nil { @@ -262,7 +262,7 @@ func TestConnectorWithProxyClass(t *testing.T) { app: kubetypes.AppConnector, } expectEqual(t, fc, expectedSecret(t, fc, opts)) - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // 2. Update Connector to specify a ProxyClass. ProxyClass is not yet // ready, so its configuration is NOT applied to the Connector @@ -271,7 +271,7 @@ func TestConnectorWithProxyClass(t *testing.T) { conn.Spec.ProxyClass = "custom-metadata" }) expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // 3. ProxyClass is set to Ready by proxy-class reconciler. Connector // get reconciled and configuration from the ProxyClass is applied to @@ -286,7 +286,7 @@ func TestConnectorWithProxyClass(t *testing.T) { }) opts.proxyClass = pc.Name expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // 4. Connector.spec.proxyClass field is unset, Connector gets // reconciled and configuration from the ProxyClass is removed from the @@ -296,7 +296,7 @@ func TestConnectorWithProxyClass(t *testing.T) { }) opts.proxyClass = "" expectReconciled(t, cr, "", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) } func TestConnectorWithAppConnector(t *testing.T) { @@ -352,7 +352,7 @@ func TestConnectorWithAppConnector(t *testing.T) { isAppConnector: true, } expectEqual(t, fc, expectedSecret(t, fc, opts)) - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // Connector's ready condition should be set to true cn.ObjectMeta.Finalizers = append(cn.ObjectMeta.Finalizers, "tailscale.com/finalizer") diff --git a/cmd/k8s-operator/ingress_test.go b/cmd/k8s-operator/ingress_test.go index dbd6961d7..aacf27d8e 100644 --- a/cmd/k8s-operator/ingress_test.go +++ b/cmd/k8s-operator/ingress_test.go @@ -71,7 +71,7 @@ func TestTailscaleIngress(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, opts)) expectEqual(t, fc, expectedHeadlessService(shortName, "ingress")) - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeResourceReqs) // 2. Ingress status gets updated with ingress proxy's MagicDNS name // once that becomes available. @@ -98,7 +98,7 @@ func TestTailscaleIngress(t *testing.T) { }) opts.shouldEnableForwardingClusterTrafficViaIngress = true expectReconciled(t, ingR, "default", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // 4. Resources get cleaned up when Ingress class is unset mustUpdate(t, fc, "default", "test", func(ing *networkingv1.Ingress) { @@ -162,7 +162,7 @@ func TestTailscaleIngressHostname(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, opts)) expectEqual(t, fc, expectedHeadlessService(shortName, "ingress")) - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeResourceReqs) // 2. Ingress proxy with capability version >= 110 does not have an HTTPS endpoint set mustUpdate(t, fc, "operator-ns", opts.secretName, func(secret *corev1.Secret) { @@ -280,7 +280,7 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, opts)) expectEqual(t, fc, expectedHeadlessService(shortName, "ingress")) - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeResourceReqs) // 2. Ingress is updated to specify a ProxyClass, ProxyClass is not yet // ready, so proxy resource configuration does not change. @@ -288,7 +288,7 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) { mak.Set(&ing.ObjectMeta.Labels, LabelProxyClass, "custom-metadata") }) expectReconciled(t, ingR, "default", "test") - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeResourceReqs) // 3. ProxyClass is set to Ready by proxy-class reconciler. Ingress get // reconciled and configuration from the ProxyClass is applied to the @@ -303,7 +303,7 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) { }) expectReconciled(t, ingR, "default", "test") opts.proxyClass = pc.Name - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeResourceReqs) // 4. tailscale.com/proxy-class label is removed from the Ingress, the // Ingress gets reconciled and the custom ProxyClass configuration is @@ -313,7 +313,7 @@ func TestTailscaleIngressWithProxyClass(t *testing.T) { }) expectReconciled(t, ingR, "default", "test") opts.proxyClass = "" - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeResourceReqs) } func TestTailscaleIngressWithServiceMonitor(t *testing.T) { @@ -608,7 +608,7 @@ func TestEmptyPath(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, opts)) expectEqual(t, fc, expectedHeadlessService(shortName, "ingress")) - expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTSUserspace(t, fc, opts), removeResourceReqs) expectEvents(t, fr, tt.expectedEvents) }) diff --git a/cmd/k8s-operator/operator_test.go b/cmd/k8s-operator/operator_test.go index 33bf23e84..ff6ba4f95 100644 --- a/cmd/k8s-operator/operator_test.go +++ b/cmd/k8s-operator/operator_test.go @@ -130,7 +130,7 @@ func TestLoadBalancerClass(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, opts)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) want.Annotations = nil want.ObjectMeta.Finalizers = []string{"tailscale.com/finalizer"} @@ -268,7 +268,7 @@ func TestTailnetTargetFQDNAnnotation(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, o)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) want := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -291,7 +291,7 @@ func TestTailnetTargetFQDNAnnotation(t *testing.T) { expectEqual(t, fc, want) expectEqual(t, fc, expectedSecret(t, fc, o)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) // Change the tailscale-target-fqdn annotation which should update the // StatefulSet @@ -380,7 +380,7 @@ func TestTailnetTargetIPAnnotation(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, o)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) want := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -403,7 +403,7 @@ func TestTailnetTargetIPAnnotation(t *testing.T) { expectEqual(t, fc, want) expectEqual(t, fc, expectedSecret(t, fc, o)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) // Change the tailscale-target-ip annotation which should update the // StatefulSet @@ -631,7 +631,7 @@ func TestAnnotations(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, o)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) want := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -737,7 +737,7 @@ func TestAnnotationIntoLB(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, o)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) // Normally the Tailscale proxy pod would come up here and write its info // into the secret. Simulate that, since it would have normally happened at @@ -781,7 +781,7 @@ func TestAnnotationIntoLB(t *testing.T) { expectReconciled(t, sr, "default", "test") // None of the proxy machinery should have changed... expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) // ... but the service should have a LoadBalancer status. want = &corev1.Service{ @@ -867,7 +867,7 @@ func TestLBIntoAnnotation(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, o)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) // Normally the Tailscale proxy pod would come up here and write its info // into the secret. Simulate that, then verify reconcile again and verify @@ -927,7 +927,7 @@ func TestLBIntoAnnotation(t *testing.T) { expectReconciled(t, sr, "default", "test") expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) want = &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ @@ -1007,7 +1007,7 @@ func TestCustomHostname(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, o)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) want := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: "test", @@ -1118,7 +1118,7 @@ func TestCustomPriorityClassName(t *testing.T) { app: kubetypes.AppIngressProxy, } - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) } func TestProxyClassForService(t *testing.T) { @@ -1188,7 +1188,7 @@ func TestProxyClassForService(t *testing.T) { } expectEqual(t, fc, expectedSecret(t, fc, opts)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // 2. The Service gets updated with tailscale.com/proxy-class label // pointing at the 'custom-metadata' ProxyClass. The ProxyClass is not @@ -1197,7 +1197,7 @@ func TestProxyClassForService(t *testing.T) { mak.Set(&svc.Labels, LabelProxyClass, "custom-metadata") }) expectReconciled(t, sr, "default", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) expectEqual(t, fc, expectedSecret(t, fc, opts)) // 3. ProxyClass is set to Ready, the Service gets reconciled by the @@ -1213,7 +1213,7 @@ func TestProxyClassForService(t *testing.T) { }) opts.proxyClass = pc.Name expectReconciled(t, sr, "default", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) expectEqual(t, fc, expectedSecret(t, fc, opts), removeAuthKeyIfExistsModifier(t)) // 4. tailscale.com/proxy-class label is removed from the Service, the @@ -1224,7 +1224,7 @@ func TestProxyClassForService(t *testing.T) { }) opts.proxyClass = "" expectReconciled(t, sr, "default", "test") - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) } func TestDefaultLoadBalancer(t *testing.T) { @@ -1280,7 +1280,7 @@ func TestDefaultLoadBalancer(t *testing.T) { clusterTargetIP: "10.20.30.40", app: kubetypes.AppIngressProxy, } - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) } func TestProxyFirewallMode(t *testing.T) { @@ -1336,7 +1336,7 @@ func TestProxyFirewallMode(t *testing.T) { clusterTargetIP: "10.20.30.40", app: kubetypes.AppIngressProxy, } - expectEqual(t, fc, expectedSTS(t, fc, o), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, o), removeResourceReqs) } func Test_isMagicDNSName(t *testing.T) { @@ -1617,7 +1617,7 @@ func Test_authKeyRemoval(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, opts)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // 2. Apply update to the Secret that imitates the proxy setting device_id. s := expectedSecret(t, fc, opts) @@ -1691,7 +1691,7 @@ func Test_externalNameService(t *testing.T) { expectEqual(t, fc, expectedSecret(t, fc, opts)) expectEqual(t, fc, expectedHeadlessService(shortName, "svc")) - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) // 2. Change the ExternalName and verify that changes get propagated. mustUpdate(t, sr, "default", "test", func(s *corev1.Service) { @@ -1699,7 +1699,7 @@ func Test_externalNameService(t *testing.T) { }) expectReconciled(t, sr, "default", "test") opts.clusterTargetDNS = "bar.com" - expectEqual(t, fc, expectedSTS(t, fc, opts), removeHashAnnotation, removeResourceReqs) + expectEqual(t, fc, expectedSTS(t, fc, opts), removeResourceReqs) } func Test_metricsResourceCreation(t *testing.T) { diff --git a/cmd/k8s-operator/proxygroup.go b/cmd/k8s-operator/proxygroup.go index e7c0590b0..0d5eff551 100644 --- a/cmd/k8s-operator/proxygroup.go +++ b/cmd/k8s-operator/proxygroup.go @@ -7,7 +7,6 @@ package main import ( "context" - "crypto/sha256" "encoding/json" "errors" "fmt" @@ -237,8 +236,7 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, pg *tsapi.Pro r.ensureAddedToGaugeForProxyGroup(pg) r.mu.Unlock() - cfgHash, err := r.ensureConfigSecretsCreated(ctx, pg, proxyClass) - if err != nil { + if err := r.ensureConfigSecretsCreated(ctx, pg, proxyClass); err != nil { return fmt.Errorf("error provisioning config Secrets: %w", err) } // State secrets are precreated so we can use the ProxyGroup CR as their owner ref. @@ -306,33 +304,10 @@ func (r *ProxyGroupReconciler) maybeProvision(ctx context.Context, pg *tsapi.Pro proxyType: string(pg.Spec.Type), } ss = applyProxyClassToStatefulSet(proxyClass, ss, cfg, logger) - capver, err := r.capVerForPG(ctx, pg, logger) - if err != nil { - return fmt.Errorf("error getting device info: %w", err) - } updateSS := func(s *appsv1.StatefulSet) { - // This is a temporary workaround to ensure that egress ProxyGroup proxies with capver older than 110 - // are restarted when tailscaled configfile contents have changed. - // This workaround ensures that: - // 1. The hash mechanism is used to trigger pod restarts for proxies below capver 110. - // 2. Proxies above capver are not unnecessarily restarted when the configfile contents change. - // 3. If the hash has alreay been set, but the capver is above 110, the old hash is preserved to avoid - // unnecessary pod restarts that could result in an update loop where capver cannot be determined for a - // restarting Pod and the hash is re-added again. - // Note that this workaround is only applied to egress ProxyGroups, because ingress ProxyGroup was added after capver 110. - // Note also that the hash annotation is only set on updates, not creation, because if the StatefulSet is - // being created, there is no need for a restart. - // TODO(irbekrm): remove this in 1.84. - hash := cfgHash - if capver >= 110 { - hash = s.Spec.Template.GetAnnotations()[podAnnotationLastSetConfigFileHash] - } s.Spec = ss.Spec - if hash != "" && pg.Spec.Type == tsapi.ProxyGroupTypeEgress { - mak.Set(&s.Spec.Template.Annotations, podAnnotationLastSetConfigFileHash, hash) - } s.ObjectMeta.Labels = ss.ObjectMeta.Labels s.ObjectMeta.Annotations = ss.ObjectMeta.Annotations @@ -449,9 +424,8 @@ func (r *ProxyGroupReconciler) deleteTailnetDevice(ctx context.Context, id tailc return nil } -func (r *ProxyGroupReconciler) ensureConfigSecretsCreated(ctx context.Context, pg *tsapi.ProxyGroup, proxyClass *tsapi.ProxyClass) (hash string, err error) { +func (r *ProxyGroupReconciler) ensureConfigSecretsCreated(ctx context.Context, pg *tsapi.ProxyGroup, proxyClass *tsapi.ProxyClass) (err error) { logger := r.logger(pg.Name) - var configSHA256Sum string for i := range pgReplicas(pg) { cfgSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -467,7 +441,7 @@ func (r *ProxyGroupReconciler) ensureConfigSecretsCreated(ctx context.Context, p logger.Debugf("Secret %s/%s already exists", cfgSecret.GetNamespace(), cfgSecret.GetName()) existingCfgSecret = cfgSecret.DeepCopy() } else if !apierrors.IsNotFound(err) { - return "", err + return err } var authKey string @@ -479,65 +453,39 @@ func (r *ProxyGroupReconciler) ensureConfigSecretsCreated(ctx context.Context, p } authKey, err = newAuthKey(ctx, r.tsClient, tags) if err != nil { - return "", err + return err } } configs, err := pgTailscaledConfig(pg, proxyClass, i, authKey, existingCfgSecret) if err != nil { - return "", fmt.Errorf("error creating tailscaled config: %w", err) + return fmt.Errorf("error creating tailscaled config: %w", err) } for cap, cfg := range configs { cfgJSON, err := json.Marshal(cfg) if err != nil { - return "", fmt.Errorf("error marshalling tailscaled config: %w", err) + return fmt.Errorf("error marshalling tailscaled config: %w", err) } mak.Set(&cfgSecret.Data, tsoperator.TailscaledConfigFileName(cap), cfgJSON) } - // The config sha256 sum is a value for a hash annotation used to trigger - // pod restarts when tailscaled config changes. Any config changes apply - // to all replicas, so it is sufficient to only hash the config for the - // first replica. - // - // In future, we're aiming to eliminate restarts altogether and have - // pods dynamically reload their config when it changes. - if i == 0 { - sum := sha256.New() - for _, cfg := range configs { - // Zero out the auth key so it doesn't affect the sha256 hash when we - // remove it from the config after the pods have all authed. Otherwise - // all the pods will need to restart immediately after authing. - cfg.AuthKey = nil - b, err := json.Marshal(cfg) - if err != nil { - return "", err - } - if _, err := sum.Write(b); err != nil { - return "", err - } - } - - configSHA256Sum = fmt.Sprintf("%x", sum.Sum(nil)) - } - if existingCfgSecret != nil { if !apiequality.Semantic.DeepEqual(existingCfgSecret, cfgSecret) { logger.Debugf("Updating the existing ProxyGroup config Secret %s", cfgSecret.Name) if err := r.Update(ctx, cfgSecret); err != nil { - return "", err + return err } } } else { logger.Debugf("Creating a new config Secret %s for the ProxyGroup", cfgSecret.Name) if err := r.Create(ctx, cfgSecret); err != nil { - return "", err + return err } } } - return configSHA256Sum, nil + return nil } // ensureAddedToGaugeForProxyGroup ensures the gauge metric for the ProxyGroup resource is updated when the ProxyGroup @@ -707,24 +655,3 @@ type nodeMetadata struct { tsID tailcfg.StableNodeID dnsName string } - -// capVerForPG returns best effort capability version for the given ProxyGroup. It attempts to find it by looking at the -// Secret + Pod for the replica with ordinal 0. Returns -1 if it is not possible to determine the capability version -// (i.e there is no Pod yet). -func (r *ProxyGroupReconciler) capVerForPG(ctx context.Context, pg *tsapi.ProxyGroup, logger *zap.SugaredLogger) (tailcfg.CapabilityVersion, error) { - metas, err := r.getNodeMetadata(ctx, pg) - if err != nil { - return -1, fmt.Errorf("error getting node metadata: %w", err) - } - if len(metas) == 0 { - return -1, nil - } - dev, err := deviceInfo(metas[0].stateSecret, metas[0].podUID, logger) - if err != nil { - return -1, fmt.Errorf("error getting device info: %w", err) - } - if dev == nil { - return -1, nil - } - return dev.capver, nil -} diff --git a/cmd/k8s-operator/proxygroup_test.go b/cmd/k8s-operator/proxygroup_test.go index f3f87aaac..c556ae94a 100644 --- a/cmd/k8s-operator/proxygroup_test.go +++ b/cmd/k8s-operator/proxygroup_test.go @@ -30,7 +30,6 @@ import ( "tailscale.com/kube/kubetypes" "tailscale.com/tstest" "tailscale.com/types/ptr" - "tailscale.com/util/mak" ) const testProxyImage = "tailscale/tailscale:test" @@ -40,7 +39,6 @@ var defaultProxyClassAnnotations = map[string]string{ } func TestProxyGroup(t *testing.T) { - const initialCfgHash = "6632726be70cf224049580deb4d317bba065915b5fd415461d60ed621c91b196" pc := &tsapi.ProxyClass{ ObjectMeta: metav1.ObjectMeta{ @@ -98,7 +96,7 @@ func TestProxyGroup(t *testing.T) { tsoperator.SetProxyGroupCondition(pg, tsapi.ProxyGroupReady, metav1.ConditionFalse, reasonProxyGroupCreating, "the ProxyGroup's ProxyClass default-pc is not yet in a ready state, waiting...", 0, cl, zl.Sugar()) expectEqual(t, fc, pg) - expectProxyGroupResources(t, fc, pg, false, "", pc) + expectProxyGroupResources(t, fc, pg, false, pc) }) t.Run("observe_ProxyGroupCreating_status_reason", func(t *testing.T) { @@ -119,11 +117,11 @@ func TestProxyGroup(t *testing.T) { tsoperator.SetProxyGroupCondition(pg, tsapi.ProxyGroupReady, metav1.ConditionFalse, reasonProxyGroupCreating, "0/2 ProxyGroup pods running", 0, cl, zl.Sugar()) expectEqual(t, fc, pg) - expectProxyGroupResources(t, fc, pg, true, "", pc) + expectProxyGroupResources(t, fc, pg, true, pc) if expected := 1; reconciler.egressProxyGroups.Len() != expected { t.Fatalf("expected %d egress ProxyGroups, got %d", expected, reconciler.egressProxyGroups.Len()) } - expectProxyGroupResources(t, fc, pg, true, "", pc) + expectProxyGroupResources(t, fc, pg, true, pc) keyReq := tailscale.KeyCapabilities{ Devices: tailscale.KeyDeviceCapabilities{ Create: tailscale.KeyDeviceCreateCapabilities{ @@ -155,7 +153,7 @@ func TestProxyGroup(t *testing.T) { } tsoperator.SetProxyGroupCondition(pg, tsapi.ProxyGroupReady, metav1.ConditionTrue, reasonProxyGroupReady, reasonProxyGroupReady, 0, cl, zl.Sugar()) expectEqual(t, fc, pg) - expectProxyGroupResources(t, fc, pg, true, initialCfgHash, pc) + expectProxyGroupResources(t, fc, pg, true, pc) }) t.Run("scale_up_to_3", func(t *testing.T) { @@ -166,7 +164,7 @@ func TestProxyGroup(t *testing.T) { expectReconciled(t, reconciler, "", pg.Name) tsoperator.SetProxyGroupCondition(pg, tsapi.ProxyGroupReady, metav1.ConditionFalse, reasonProxyGroupCreating, "2/3 ProxyGroup pods running", 0, cl, zl.Sugar()) expectEqual(t, fc, pg) - expectProxyGroupResources(t, fc, pg, true, initialCfgHash, pc) + expectProxyGroupResources(t, fc, pg, true, pc) addNodeIDToStateSecrets(t, fc, pg) expectReconciled(t, reconciler, "", pg.Name) @@ -176,7 +174,7 @@ func TestProxyGroup(t *testing.T) { TailnetIPs: []string{"1.2.3.4", "::1"}, }) expectEqual(t, fc, pg) - expectProxyGroupResources(t, fc, pg, true, initialCfgHash, pc) + expectProxyGroupResources(t, fc, pg, true, pc) }) t.Run("scale_down_to_1", func(t *testing.T) { @@ -189,21 +187,7 @@ func TestProxyGroup(t *testing.T) { pg.Status.Devices = pg.Status.Devices[:1] // truncate to only the first device. expectEqual(t, fc, pg) - expectProxyGroupResources(t, fc, pg, true, initialCfgHash, pc) - }) - - t.Run("trigger_config_change_and_observe_new_config_hash", func(t *testing.T) { - pc.Spec.TailscaleConfig = &tsapi.TailscaleConfig{ - AcceptRoutes: true, - } - mustUpdate(t, fc, "", pc.Name, func(p *tsapi.ProxyClass) { - p.Spec = pc.Spec - }) - - expectReconciled(t, reconciler, "", pg.Name) - - expectEqual(t, fc, pg) - expectProxyGroupResources(t, fc, pg, true, "518a86e9fae64f270f8e0ec2a2ea6ca06c10f725035d3d6caca132cd61e42a74", pc) + expectProxyGroupResources(t, fc, pg, true, pc) }) t.Run("enable_metrics", func(t *testing.T) { @@ -608,7 +592,7 @@ func verifyEnvVarNotPresent(t *testing.T, sts *appsv1.StatefulSet, name string) } } -func expectProxyGroupResources(t *testing.T, fc client.WithWatch, pg *tsapi.ProxyGroup, shouldExist bool, cfgHash string, proxyClass *tsapi.ProxyClass) { +func expectProxyGroupResources(t *testing.T, fc client.WithWatch, pg *tsapi.ProxyGroup, shouldExist bool, proxyClass *tsapi.ProxyClass) { t.Helper() role := pgRole(pg, tsNamespace) @@ -619,9 +603,6 @@ func expectProxyGroupResources(t *testing.T, fc client.WithWatch, pg *tsapi.Prox t.Fatal(err) } statefulSet.Annotations = defaultProxyClassAnnotations - if cfgHash != "" { - mak.Set(&statefulSet.Spec.Template.Annotations, podAnnotationLastSetConfigFileHash, cfgHash) - } if shouldExist { expectEqual(t, fc, role) diff --git a/cmd/k8s-operator/sts.go b/cmd/k8s-operator/sts.go index 70b25f2d2..4c7c3ac67 100644 --- a/cmd/k8s-operator/sts.go +++ b/cmd/k8s-operator/sts.go @@ -7,7 +7,6 @@ package main import ( "context" - "crypto/sha256" _ "embed" "encoding/json" "errors" @@ -91,8 +90,6 @@ const ( podAnnotationLastSetClusterDNSName = "tailscale.com/operator-last-set-cluster-dns-name" podAnnotationLastSetTailnetTargetIP = "tailscale.com/operator-last-set-ts-tailnet-target-ip" podAnnotationLastSetTailnetTargetFQDN = "tailscale.com/operator-last-set-ts-tailnet-target-fqdn" - // podAnnotationLastSetConfigFileHash is sha256 hash of the current tailscaled configuration contents. - podAnnotationLastSetConfigFileHash = "tailscale.com/operator-last-set-config-file-hash" proxyTypeEgress = "egress_service" proxyTypeIngressService = "ingress_service" @@ -110,7 +107,7 @@ var ( // tailscaleManagedLabels are label keys that tailscale operator sets on StatefulSets and Pods. tailscaleManagedLabels = []string{kubetypes.LabelManaged, LabelParentType, LabelParentName, LabelParentNamespace, "app"} // tailscaleManagedAnnotations are annotation keys that tailscale operator sets on StatefulSets and Pods. - tailscaleManagedAnnotations = []string{podAnnotationLastSetClusterIP, podAnnotationLastSetTailnetTargetIP, podAnnotationLastSetTailnetTargetFQDN, podAnnotationLastSetConfigFileHash} + tailscaleManagedAnnotations = []string{podAnnotationLastSetClusterIP, podAnnotationLastSetTailnetTargetIP, podAnnotationLastSetTailnetTargetFQDN} ) type tailscaleSTSConfig struct { @@ -201,11 +198,11 @@ func (a *tailscaleSTSReconciler) Provision(ctx context.Context, logger *zap.Suga } sts.ProxyClass = proxyClass - secretName, tsConfigHash, _, err := a.createOrGetSecret(ctx, logger, sts, hsvc) + secretName, _, err := a.createOrGetSecret(ctx, logger, sts, hsvc) if err != nil { return nil, fmt.Errorf("failed to create or get API key secret: %w", err) } - _, err = a.reconcileSTS(ctx, logger, sts, hsvc, secretName, tsConfigHash) + _, err = a.reconcileSTS(ctx, logger, sts, hsvc, secretName) if err != nil { return nil, fmt.Errorf("failed to reconcile statefulset: %w", err) } @@ -335,7 +332,7 @@ func (a *tailscaleSTSReconciler) reconcileHeadlessService(ctx context.Context, l return createOrUpdate(ctx, a.Client, a.operatorNamespace, hsvc, func(svc *corev1.Service) { svc.Spec = hsvc.Spec }) } -func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger *zap.SugaredLogger, stsC *tailscaleSTSConfig, hsvc *corev1.Service) (secretName, hash string, configs tailscaledConfigs, _ error) { +func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger *zap.SugaredLogger, stsC *tailscaleSTSConfig, hsvc *corev1.Service) (secretName string, configs tailscaledConfigs, _ error) { secret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ // Hardcode a -0 suffix so that in future, if we support @@ -351,7 +348,7 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger * logger.Debugf("secret %s/%s already exists", secret.GetNamespace(), secret.GetName()) orig = secret.DeepCopy() } else if !apierrors.IsNotFound(err) { - return "", "", nil, err + return "", nil, err } var authKey string @@ -361,13 +358,13 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger * // ACME account key. sts, err := getSingleObject[appsv1.StatefulSet](ctx, a.Client, a.operatorNamespace, stsC.ChildResourceLabels) if err != nil { - return "", "", nil, err + return "", nil, err } if sts != nil { // StatefulSet exists, so we have already created the secret. // If the secret is missing, they should delete the StatefulSet. logger.Errorf("Tailscale proxy secret doesn't exist, but the corresponding StatefulSet %s/%s already does. Something is wrong, please delete the StatefulSet.", sts.GetNamespace(), sts.GetName()) - return "", "", nil, nil + return "", nil, nil } // Create API Key secret which is going to be used by the statefulset // to authenticate with Tailscale. @@ -378,25 +375,20 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger * } authKey, err = newAuthKey(ctx, a.tsClient, tags) if err != nil { - return "", "", nil, err + return "", nil, err } } configs, err := tailscaledConfig(stsC, authKey, orig) if err != nil { - return "", "", nil, fmt.Errorf("error creating tailscaled config: %w", err) + return "", nil, fmt.Errorf("error creating tailscaled config: %w", err) } - hash, err = tailscaledConfigHash(configs) - if err != nil { - return "", "", nil, fmt.Errorf("error calculating hash of tailscaled configs: %w", err) - } - latest := tailcfg.CapabilityVersion(-1) var latestConfig ipn.ConfigVAlpha for key, val := range configs { fn := tsoperator.TailscaledConfigFileName(key) b, err := json.Marshal(val) if err != nil { - return "", "", nil, fmt.Errorf("error marshalling tailscaled config: %w", err) + return "", nil, fmt.Errorf("error marshalling tailscaled config: %w", err) } mak.Set(&secret.StringData, fn, string(b)) if key > latest { @@ -408,7 +400,7 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger * if stsC.ServeConfig != nil { j, err := json.Marshal(stsC.ServeConfig) if err != nil { - return "", "", nil, err + return "", nil, err } mak.Set(&secret.StringData, "serve-config", string(j)) } @@ -416,15 +408,15 @@ func (a *tailscaleSTSReconciler) createOrGetSecret(ctx context.Context, logger * if orig != nil { logger.Debugf("patching the existing proxy Secret with tailscaled config %s", sanitizeConfigBytes(latestConfig)) if err := a.Patch(ctx, secret, client.MergeFrom(orig)); err != nil { - return "", "", nil, err + return "", nil, err } } else { logger.Debugf("creating a new Secret for the proxy with tailscaled config %s", sanitizeConfigBytes(latestConfig)) if err := a.Create(ctx, secret); err != nil { - return "", "", nil, err + return "", nil, err } } - return secret.Name, hash, configs, nil + return secret.Name, configs, nil } // sanitizeConfigBytes returns ipn.ConfigVAlpha in string form with redacted @@ -535,7 +527,7 @@ var proxyYaml []byte //go:embed deploy/manifests/userspace-proxy.yaml var userspaceProxyYaml []byte -func (a *tailscaleSTSReconciler) reconcileSTS(ctx context.Context, logger *zap.SugaredLogger, sts *tailscaleSTSConfig, headlessSvc *corev1.Service, proxySecret, tsConfigHash string) (*appsv1.StatefulSet, error) { +func (a *tailscaleSTSReconciler) reconcileSTS(ctx context.Context, logger *zap.SugaredLogger, sts *tailscaleSTSConfig, headlessSvc *corev1.Service, proxySecret string) (*appsv1.StatefulSet, error) { ss := new(appsv1.StatefulSet) if sts.ServeConfig != nil && sts.ForwardClusterTrafficViaL7IngressProxy != true { // If forwarding cluster traffic via is required we need non-userspace + NET_ADMIN + forwarding if err := yaml.Unmarshal(userspaceProxyYaml, &ss); err != nil { @@ -662,11 +654,6 @@ func (a *tailscaleSTSReconciler) reconcileSTS(ctx context.Context, logger *zap.S }) } - dev, err := a.DeviceInfo(ctx, sts.ChildResourceLabels, logger) - if err != nil { - return nil, fmt.Errorf("failed to get device info: %w", err) - } - app, err := appInfoForProxy(sts) if err != nil { // No need to error out if now or in future we end up in a @@ -685,25 +672,7 @@ func (a *tailscaleSTSReconciler) reconcileSTS(ctx context.Context, logger *zap.S ss = applyProxyClassToStatefulSet(sts.ProxyClass, ss, sts, logger) } updateSS := func(s *appsv1.StatefulSet) { - // This is a temporary workaround to ensure that proxies with capver older than 110 - // are restarted when tailscaled configfile contents have changed. - // This workaround ensures that: - // 1. The hash mechanism is used to trigger pod restarts for proxies below capver 110. - // 2. Proxies above capver are not unnecessarily restarted when the configfile contents change. - // 3. If the hash has alreay been set, but the capver is above 110, the old hash is preserved to avoid - // unnecessary pod restarts that could result in an update loop where capver cannot be determined for a - // restarting Pod and the hash is re-added again. - // Note that the hash annotation is only set on updates not creation, because if the StatefulSet is - // being created, there is no need for a restart. - // TODO(irbekrm): remove this in 1.84. - hash := tsConfigHash - if dev == nil || dev.capver >= 110 { - hash = s.Spec.Template.GetAnnotations()[podAnnotationLastSetConfigFileHash] - } s.Spec = ss.Spec - if hash != "" { - mak.Set(&s.Spec.Template.Annotations, podAnnotationLastSetConfigFileHash, hash) - } s.ObjectMeta.Labels = ss.Labels s.ObjectMeta.Annotations = ss.Annotations } @@ -937,8 +906,7 @@ func readAuthKey(secret *corev1.Secret, key string) (*string, error) { } // tailscaledConfig takes a proxy config, a newly generated auth key if generated and a Secret with the previous proxy -// state and auth key and returns tailscaled config files for currently supported proxy versions and a hash of that -// configuration. +// state and auth key and returns tailscaled config files for currently supported proxy versions. func tailscaledConfig(stsC *tailscaleSTSConfig, newAuthkey string, oldSecret *corev1.Secret) (tailscaledConfigs, error) { conf := &ipn.ConfigVAlpha{ Version: "alpha0", @@ -1031,27 +999,6 @@ type ptrObject[T any] interface { type tailscaledConfigs map[tailcfg.CapabilityVersion]ipn.ConfigVAlpha -// hashBytes produces a hash for the provided tailscaled config that is the same across -// different invocations of this code. We do not use the -// tailscale.com/deephash.Hash here because that produces a different hash for -// the same value in different tailscale builds. The hash we are producing here -// is used to determine if the container running the Connector Tailscale node -// needs to be restarted. The container does not need restarting when the only -// thing that changed is operator version (the hash is also exposed to users via -// an annotation and might be confusing if it changes without the config having -// changed). -func tailscaledConfigHash(c tailscaledConfigs) (string, error) { - b, err := json.Marshal(c) - if err != nil { - return "", fmt.Errorf("error marshalling tailscaled configs: %w", err) - } - h := sha256.New() - if _, err = h.Write(b); err != nil { - return "", fmt.Errorf("error calculating hash: %w", err) - } - return fmt.Sprintf("%x", h.Sum(nil)), nil -} - // createOrMaybeUpdate adds obj to the k8s cluster, unless the object already exists, // in which case update is called to make changes to it. If update is nil or returns // an error, the object is returned unmodified. diff --git a/cmd/k8s-operator/testutils_test.go b/cmd/k8s-operator/testutils_test.go index 619aecc56..56542700d 100644 --- a/cmd/k8s-operator/testutils_test.go +++ b/cmd/k8s-operator/testutils_test.go @@ -62,7 +62,6 @@ type configOpts struct { subnetRoutes string isExitNode bool isAppConnector bool - confFileHash string serveConfig *ipn.ServeConfig shouldEnableForwardingClusterTrafficViaIngress bool proxyClass string // configuration from the named ProxyClass should be applied to proxy resources @@ -120,9 +119,6 @@ func expectedSTS(t *testing.T, cl client.Client, opts configOpts) *appsv1.Statef ReadOnly: true, MountPath: "/etc/tsconfig", }} - if opts.confFileHash != "" { - mak.Set(&annots, "tailscale.com/operator-last-set-config-file-hash", opts.confFileHash) - } if opts.firewallMode != "" { tsContainer.Env = append(tsContainer.Env, corev1.EnvVar{ Name: "TS_DEBUG_FIREWALL_MODE", @@ -358,10 +354,6 @@ func expectedSTSUserspace(t *testing.T, cl client.Client, opts configOpts) *apps }, }, } - ss.Spec.Template.Annotations = map[string]string{} - if opts.confFileHash != "" { - ss.Spec.Template.Annotations["tailscale.com/operator-last-set-config-file-hash"] = opts.confFileHash - } // If opts.proxyClass is set, retrieve the ProxyClass and apply // configuration from that to the StatefulSet. if opts.proxyClass != "" { @@ -842,17 +834,6 @@ func (c *fakeTSClient) Deleted() []string { return c.deleted } -// removeHashAnnotation can be used to remove declarative tailscaled config hash -// annotation from proxy StatefulSets to make the tests more maintainable (so -// that we don't have to change the annotation in each test case after any -// change to the configfile contents). -func removeHashAnnotation(sts *appsv1.StatefulSet) { - delete(sts.Spec.Template.Annotations, podAnnotationLastSetConfigFileHash) - if len(sts.Spec.Template.Annotations) == 0 { - sts.Spec.Template.Annotations = nil - } -} - func removeResourceReqs(sts *appsv1.StatefulSet) { if sts != nil { sts.Spec.Template.Spec.Resources = nil