diff --git a/api/v1alpha1/bindcluster_types.go b/api/v1alpha1/bindcluster_types.go index 9fc5518..270fc80 100644 --- a/api/v1alpha1/bindcluster_types.go +++ b/api/v1alpha1/bindcluster_types.go @@ -158,6 +158,11 @@ type BindClusterStatus struct { // +optional ObservedGeneration int64 `json:"observedGeneration,omitempty"` + // AppliedConfig maps each pod to the hash of the config it last loaded via + // rndc reconfig. + // +optional + AppliedConfig map[string]string `json:"appliedConfig,omitempty"` + // Conditions represent the latest available observations. // +optional // +listType=map diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index c9cb4cc..7db68b4 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -340,6 +340,13 @@ func (in *BindClusterSpec) DeepCopy() *BindClusterSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BindClusterStatus) DeepCopyInto(out *BindClusterStatus) { *out = *in + if in.AppliedConfig != nil { + in, out := &in.AppliedConfig, &out.AppliedConfig + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.Conditions != nil { in, out := &in.Conditions, &out.Conditions *out = make([]v1.Condition, len(*in)) diff --git a/config/crd/bases/bind.unkin.net_bindclusters.yaml b/config/crd/bases/bind.unkin.net_bindclusters.yaml index 759e14e..4b96801 100644 --- a/config/crd/bases/bind.unkin.net_bindclusters.yaml +++ b/config/crd/bases/bind.unkin.net_bindclusters.yaml @@ -1206,6 +1206,13 @@ spec: status: description: BindClusterStatus reports observed cluster state. properties: + appliedConfig: + additionalProperties: + type: string + description: |- + AppliedConfig maps each pod to the hash of the config it last loaded via + rndc reconfig. + type: object conditions: description: Conditions represent the latest available observations. items: diff --git a/config/crd/install.yaml b/config/crd/install.yaml index 19d3b03..29a9f1d 100644 --- a/config/crd/install.yaml +++ b/config/crd/install.yaml @@ -1516,6 +1516,13 @@ spec: status: description: BindClusterStatus reports observed cluster state. properties: + appliedConfig: + additionalProperties: + type: string + description: |- + AppliedConfig maps each pod to the hash of the config it last loaded via + rndc reconfig. + type: object conditions: description: Conditions represent the latest available observations. items: diff --git a/internal/bind/consts.go b/internal/bind/consts.go index b574fc0..931aed7 100644 --- a/internal/bind/consts.go +++ b/internal/bind/consts.go @@ -20,10 +20,11 @@ const ( RunDir = "/run/named" // Binary locations in the ISC BIND9 image (Debian/Ubuntu layout). - NamedBin = "/usr/sbin/named" - RndcBin = "/usr/sbin/rndc" - NsupdateBin = "/usr/bin/nsupdate" - DigBin = "/usr/bin/dig" + NamedBin = "/usr/sbin/named" + RndcBin = "/usr/sbin/rndc" + NsupdateBin = "/usr/bin/nsupdate" + DigBin = "/usr/bin/dig" + CheckconfBin = "/usr/bin/named-checkconf" ) // Config file paths derived from ConfigDir. diff --git a/internal/bind/render.go b/internal/bind/render.go index 0c03a37..b00c680 100644 --- a/internal/bind/render.go +++ b/internal/bind/render.go @@ -32,8 +32,8 @@ type RenderInput struct { // also-notify entries, applied via rndc, carry `key ""`), and // secondaries render `allow-notify { key ""; }` — an address-match-list // with a *key* element and NO IPs. Because it names only the key, this clause - // is fully static: it never changes when a pod IP churns, so it cannot feed a - // changed config-hash and cannot roll the StatefulSet (the v0.2.5 loop). Empty + // is fully static: it never changes when a pod IP churns, so pod churn cannot + // re-render named.conf (the v0.2.5 loop). Empty // leaves BIND's default behaviour unchanged. NotifyKeyName string } @@ -310,10 +310,10 @@ func transferPrimaries(in RenderInput) []string { // // The rendered clause is `allow-notify { key ""; };` — a key element with // NO IP addresses. It is therefore fully static: it depends only on the key -// name, never on any pod IP, so it can never change the config-hash and can -// never trigger a rolling restart. This is the fix for the v0.2.5 loop, where an -// options-scope allow-notify enumerating the primary pod IP re-rendered on every -// pod churn, flipped the hash, rolled the pods, changed the IP, and looped. +// name, never on any pod IP, so pod churn never re-renders named.conf. This is +// the fix for the v0.2.5 loop, where an options-scope allow-notify enumerating +// the primary pod IP re-rendered on every pod churn, flipped the hash, rolled +// the pods, changed the IP, and looped. // // Emitted only on secondaries and only when the NOTIFY key name is known. func allowNotifyClause(in RenderInput, isPrimary bool, indent string) string { diff --git a/internal/controller/bindcluster_controller.go b/internal/controller/bindcluster_controller.go index 8cf8c56..b4c4279 100644 --- a/internal/controller/bindcluster_controller.go +++ b/internal/controller/bindcluster_controller.go @@ -1,17 +1,18 @@ package controller import ( - "bytes" "context" "crypto/sha256" "encoding/hex" "fmt" "sort" "strings" + "time" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -32,7 +33,13 @@ import ( type BindClusterReconciler struct { client.Client Scheme *runtime.Scheme - Exec *bind.Executor + Exec podExecutor +} + +// podExecutor is the subset of *bind.Executor the cluster reconciler uses. +type podExecutor interface { + Exec(ctx context.Context, namespace, pod string, command []string, stdin string) (string, error) + Reconfig(ctx context.Context, namespace, pod string) error } // +kubebuilder:rbac:groups=bind.unkin.net,resources=bindclusters,verbs=get;list;watch;create;update;patch;delete @@ -54,10 +61,12 @@ func (r *BindClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) if err := r.reconcileRNDCSecret(ctx, &cluster); err != nil { return ctrl.Result{}, fmt.Errorf("rndc secret: %w", err) } - if err := r.reconcileKeysSecret(ctx, &cluster); err != nil { + keysConf, err := r.reconcileKeysSecret(ctx, &cluster) + if err != nil { return ctrl.Result{}, fmt.Errorf("keys secret: %w", err) } - if err := r.reconcileConfigMap(ctx, &cluster); err != nil { + primaryConf, secondaryConf, err := r.reconcileConfigMap(ctx, &cluster) + if err != nil { return ctrl.Result{}, fmt.Errorf("configmap: %w", err) } if err := r.reconcileServices(ctx, &cluster); err != nil { @@ -68,9 +77,7 @@ func (r *BindClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, fmt.Errorf("statefulset: %w", err) } - // Best-effort: reload configuration on ready pods so ConfigMap changes take - // effect without a rollout. - r.reloadReadyPods(ctx, &cluster) + requeue := r.reconfigPods(ctx, &cluster, primaryConf, secondaryConf, keysConf) // Status. cluster.Status.ObservedGeneration = cluster.Generation @@ -91,7 +98,10 @@ func (r *BindClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) } if !ready { - return ctrl.Result{RequeueAfter: requeueShort}, nil + requeue = requeueShort + } + if requeue > 0 { + return ctrl.Result{RequeueAfter: requeue}, nil } logger.V(1).Info("cluster reconciled", "cluster", cluster.Name, "ready", sts.Status.ReadyReplicas) return ctrl.Result{}, nil @@ -126,10 +136,10 @@ func (r *BindClusterReconciler) reconcileRNDCSecret(ctx context.Context, c *bind return r.Create(ctx, s) } -func (r *BindClusterReconciler) reconcileKeysSecret(ctx context.Context, c *bindv1alpha1.BindCluster) error { +func (r *BindClusterReconciler) reconcileKeysSecret(ctx context.Context, c *bindv1alpha1.BindCluster) (string, error) { var keys bindv1alpha1.BindTSIGKeyList if err := r.List(ctx, &keys, client.InNamespace(c.Namespace)); err != nil { - return err + return "", err } // Include keys scoped to this cluster (spec.clusterRef == name) and shared // keys (empty clusterRef). This keeps keys from leaking across clusters that @@ -167,10 +177,10 @@ func (r *BindClusterReconciler) reconcileKeysSecret(ctx context.Context, c *bind b.WriteString(bind.KeyClause(keyName, alg, string(secret.Data["secret"]))) } - return r.upsertSecret(ctx, c, keysSecretName(c.Name), map[string][]byte{"keys.conf": []byte(b.String())}) + return b.String(), r.upsertSecret(ctx, c, keysSecretName(c.Name), map[string][]byte{"keys.conf": []byte(b.String())}) } -func (r *BindClusterReconciler) reconcileConfigMap(ctx context.Context, c *bindv1alpha1.BindCluster) error { +func (r *BindClusterReconciler) reconcileConfigMap(ctx context.Context, c *bindv1alpha1.BindCluster) (primaryConf, secondaryConf string, err error) { // BIND primaries/default-primaries need an IP address, not a DNS name. Use // the stable primary Service ClusterIP so secondaries keep transferring // across primary pod restarts (falls back to the pod IP when no primary @@ -241,13 +251,13 @@ func (r *BindClusterReconciler) reconcileConfigMap(ctx context.Context, c *bindv in.NotifyKeyName = tsigKeyName(ctx, r.Client, c.Namespace, in.Catalog.Spec.TransferKeyRef) } - primaryConf, secondaryConf := bind.RenderNamedConf(in) + primaryConf, secondaryConf = bind.RenderNamedConf(in) data := map[string]string{ "named.conf.primary": primaryConf, "named.conf.secondary": secondaryConf, "entrypoint.sh": entrypointScript(), } - return r.upsertConfigMap(ctx, c, configMapName(c.Name), data) + return primaryConf, secondaryConf, r.upsertConfigMap(ctx, c, configMapName(c.Name), data) } func (r *BindClusterReconciler) reconcileServices(ctx context.Context, c *bindv1alpha1.BindCluster) error { @@ -364,12 +374,8 @@ func (r *BindClusterReconciler) reconcileStatefulSet(ctx context.Context, c *bin }}}, } - // Pods copy config from the projected volume into an emptyDir once at - // startup; a ConfigMap/keys change never reaches a running pod (rndc - // reconfig re-reads the stale startup copy). Stamp a hash of the projected - // config onto the pod template so a config change rolls the StatefulSet, - // which is the only way the change takes effect. The operator owns the - // template, so this restart is operator-driven and not reverted. + // Rendered config is applied in place by reconfigPods; only an entrypoint + // change needs a restart, so only its hash goes on the template. sts := &appsv1.StatefulSet{ ObjectMeta: metav1.ObjectMeta{Name: c.Name, Namespace: c.Namespace, Labels: labels}, Spec: appsv1.StatefulSetSpec{ @@ -377,7 +383,7 @@ func (r *BindClusterReconciler) reconcileStatefulSet(ctx context.Context, c *bin Replicas: &replicas, Selector: &metav1.LabelSelector{MatchLabels: labels}, Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{Labels: labels, Annotations: map[string]string{configHashAnnotation: r.configHash(ctx, c)}}, + ObjectMeta: metav1.ObjectMeta{Labels: labels, Annotations: map[string]string{entrypointHashAnnotation: sha256Hex(entrypointScript())}}, Spec: corev1.PodSpec{ NodeSelector: c.Spec.NodeSelector, Tolerations: c.Spec.Tolerations, @@ -445,63 +451,71 @@ func (r *BindClusterReconciler) reconcileStatefulSet(ctx context.Context, c *bin return &existing, nil } -// configHashAnnotation carries a hash of the projected config on the pod -// template; changing it triggers a rolling restart so pods pick up new config. -const configHashAnnotation = "bind.unkin.net/config-hash" +// entrypointHashAnnotation rolls the pods when the entrypoint script changes. +const entrypointHashAnnotation = "bind.unkin.net/entrypoint-hash" -// configHash returns a deterministic hash of the config projected into the pods -// (the rendered ConfigMap and the keys.conf Secret). It is read after those are -// reconciled, so it reflects the current desired config. A stable hash means no -// spurious restarts; any config or TSIG-key change flips it and rolls the pods. -func (r *BindClusterReconciler) configHash(ctx context.Context, c *bindv1alpha1.BindCluster) string { - var buf bytes.Buffer - var cm corev1.ConfigMap - if err := r.Get(ctx, types.NamespacedName{Namespace: c.Namespace, Name: configMapName(c.Name)}, &cm); err == nil { - for _, k := range sortedKeys(cm.Data) { - fmt.Fprintf(&buf, "%s\x00%s\x00", k, cm.Data[k]) - } - } - var keys corev1.Secret - if err := r.Get(ctx, types.NamespacedName{Namespace: c.Namespace, Name: keysSecretName(c.Name)}, &keys); err == nil { - data := make(map[string]string, len(keys.Data)) - for k, v := range keys.Data { - data[k] = string(v) - } - for _, k := range sortedKeys(data) { - fmt.Fprintf(&buf, "%s\x00%s\x00", k, data[k]) - } - } - sum := sha256.Sum256(buf.Bytes()) +func sha256Hex(s string) string { + sum := sha256.Sum256([]byte(s)) return hex.EncodeToString(sum[:]) } -func sortedKeys(m map[string]string) []string { - keys := make([]string, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - sort.Strings(keys) - return keys -} - -func (r *BindClusterReconciler) reloadReadyPods(ctx context.Context, c *bindv1alpha1.BindCluster) { +// reconfigPods runs rndc reconfig on each ready pod whose applied config is not +// the desired one, once kubelet has propagated it to the pod's mount (eventually +// consistent, up to ~90s). It records applied hashes in status and sets the +// ConfigApplied condition, returning a requeue delay while any pod is pending. +func (r *BindClusterReconciler) reconfigPods(ctx context.Context, c *bindv1alpha1.BindCluster, primaryConf, secondaryConf, keysConf string) time.Duration { if r.Exec == nil { - return + return 0 } - logger := log.FromContext(ctx) var pods corev1.PodList if err := r.List(ctx, &pods, client.InNamespace(c.Namespace), client.MatchingLabels(commonLabels(c.Name))); err != nil { - return + return requeueShort } + applied := map[string]string{} + var waiting, failed []string for i := range pods.Items { pod := &pods.Items[i] - if !podReady(pod) { + conf := secondaryConf + if pod.Name == primaryPodName(c.Name) { + conf = primaryConf + } + want := sha256Hex(conf + keysConf) + if have, ok := c.Status.AppliedConfig[pod.Name]; ok { + applied[pod.Name] = have + } + if applied[pod.Name] == want || !podReady(pod) { + continue + } + mounted, err := r.Exec.Exec(ctx, c.Namespace, pod.Name, []string{"cat", bind.NamedConfRun, bind.KeysConfPath}, "") + if err != nil || sha256Hex(mounted) != want { + waiting = append(waiting, pod.Name) continue } if err := r.Exec.Reconfig(ctx, c.Namespace, pod.Name); err != nil { - logger.V(1).Info("rndc reconfig failed", "pod", pod.Name, "err", err.Error()) + // rndc only reports "failed: "; checkconf names the file and line. + detail, _ := r.Exec.Exec(ctx, c.Namespace, pod.Name, []string{bind.CheckconfBin, bind.NamedConfRun}, "") + failed = append(failed, strings.TrimSpace(fmt.Sprintf("%s: %v %s", pod.Name, err, detail))) + continue } + applied[pod.Name] = want } + c.Status.AppliedConfig = applied + + cond := metav1.Condition{Type: "ConfigApplied", Status: metav1.ConditionTrue, Reason: "Applied", ObservedGeneration: c.Generation} + var requeue time.Duration + switch { + case len(failed) > 0: + cond.Status, cond.Reason, cond.Message = metav1.ConditionFalse, "ReconfigFailed", strings.Join(failed, "; ") + requeue = requeueLong + log.FromContext(ctx).Info("rndc reconfig failed", "cluster", c.Name, "errors", cond.Message) + case len(waiting) > 0: + cond.Status, cond.Reason, cond.Message = metav1.ConditionFalse, "Propagating", "awaiting config on "+strings.Join(waiting, ", ") + } + if len(waiting) > 0 { + requeue = requeueShort + } + meta.SetStatusCondition(&c.Status.Conditions, cond) + return requeue } func (r *BindClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { diff --git a/internal/controller/config_hash_test.go b/internal/controller/config_hash_test.go deleted file mode 100644 index ebb4a27..0000000 --- a/internal/controller/config_hash_test.go +++ /dev/null @@ -1,64 +0,0 @@ -package controller - -import ( - "context" - "testing" - - corev1 "k8s.io/api/core/v1" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/runtime" - clientgoscheme "k8s.io/client-go/kubernetes/scheme" - "sigs.k8s.io/controller-runtime/pkg/client/fake" - - bindv1alpha1 "git.unkin.net/unkin/bind-operator/api/v1alpha1" -) - -func TestConfigHashStableAndSensitive(t *testing.T) { - scheme := runtime.NewScheme() - if err := clientgoscheme.AddToScheme(scheme); err != nil { - t.Fatal(err) - } - if err := bindv1alpha1.AddToScheme(scheme); err != nil { - t.Fatal(err) - } - cluster := &bindv1alpha1.BindCluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: "ns"}} - cm := &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{Name: configMapName("c"), Namespace: "ns"}, - Data: map[string]string{"named.conf.secondary": "options { recursion yes; };"}, - } - keys := &corev1.Secret{ - ObjectMeta: metav1.ObjectMeta{Name: keysSecretName("c"), Namespace: "ns"}, - Data: map[string][]byte{"keys.conf": []byte("key x {};")}, - } - c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cm, keys).Build() - r := &BindClusterReconciler{Client: c, Scheme: scheme} - ctx := context.Background() - - h1 := r.configHash(ctx, cluster) - if h1 == "" { - t.Fatal("hash should not be empty when config exists") - } - // Stable across calls when nothing changes. - if h2 := r.configHash(ctx, cluster); h2 != h1 { - t.Fatalf("hash not stable: %s != %s", h1, h2) - } - - // A config change flips the hash (this is what rolls the StatefulSet). - cm.Data["named.conf.secondary"] = "options { recursion yes; validate-except { unkin.net; }; };" - if err := c.Update(ctx, cm); err != nil { - t.Fatal(err) - } - if h3 := r.configHash(ctx, cluster); h3 == h1 { - t.Fatal("hash must change when the ConfigMap changes") - } - - // A TSIG key (keys.conf) change also flips it. - afterCM := r.configHash(ctx, cluster) - keys.Data["keys.conf"] = []byte("key x { algorithm hmac-sha256; };") - if err := c.Update(ctx, keys); err != nil { - t.Fatal(err) - } - if h4 := r.configHash(ctx, cluster); h4 == afterCM { - t.Fatal("hash must change when keys.conf changes") - } -} diff --git a/internal/controller/config_hash_podip_test.go b/internal/controller/config_podip_test.go similarity index 57% rename from internal/controller/config_hash_podip_test.go rename to internal/controller/config_podip_test.go index d79e6d9..02c6456 100644 --- a/internal/controller/config_hash_podip_test.go +++ b/internal/controller/config_podip_test.go @@ -15,23 +15,11 @@ import ( bindv1alpha1 "git.unkin.net/unkin/bind-operator/api/v1alpha1" ) -// TestConfigHashIndependentOfPrimaryPodIP is the permanent regression guard for -// the v0.2.5 rolling-restart loop. -// -// v0.2.5 rendered an options-scope `allow-notify { ; ... }` into -// the (restart-scoped) named.conf. The config-hash annotation that rolls the -// StatefulSet is computed over that render (BindClusterReconciler.configHash), so -// a config change rolled the pods, the primary pod came back on a NEW IP, the -// operator re-rendered with the new IP, the hash changed, the pods rolled again, -// and so on — an infinite roll loop across every BindCluster. -// -// The fix removed all pod IPs from restart-scoped config (secondaries now admit -// intra-cluster NOTIFYs by TSIG key: `allow-notify { key "X"; }`). This test -// reconciles the ConfigMap with the primary pod on one IP, computes the hash, -// then does it again with the primary pod on a DIFFERENT IP, and asserts the -// hash is byte-identical. If any pod-IP dependency ever creeps back into -// restart-scoped config, this fails and the loop-class bug is caught. -func TestConfigHashIndependentOfPrimaryPodIP(t *testing.T) { +// TestRenderedConfigIndependentOfPrimaryPodIP guards the v0.2.5 loop: if a pod +// IP leaks into named.conf, every pod churn re-renders config and reconfigs every +// pod. Render with the primary pod on two different IPs and require identical +// output. +func TestRenderedConfigIndependentOfPrimaryPodIP(t *testing.T) { scheme := runtime.NewScheme() if err := clientgoscheme.AddToScheme(scheme); err != nil { t.Fatal(err) @@ -68,9 +56,7 @@ func TestConfigHashIndependentOfPrimaryPodIP(t *testing.T) { Spec: bindv1alpha1.BindTSIGKeySpec{ClusterRef: cluster.Name}, } - // hashWithPrimaryIP reconciles the ConfigMap with the primary pod carrying the - // given IP, then returns the resulting config-hash. - hashWithPrimaryIP := func(ip string) string { + renderWithPrimaryIP := func(ip string) string { primaryPod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Name: primaryPodName(cluster.Name), Namespace: ns, Labels: commonLabels(cluster.Name)}, Status: corev1.PodStatus{PodIP: ip}, @@ -81,7 +67,8 @@ func TestConfigHashIndependentOfPrimaryPodIP(t *testing.T) { Build() r := &BindClusterReconciler{Client: c, Scheme: scheme} ctx := context.Background() - if err := r.reconcileConfigMap(ctx, cluster); err != nil { + primaryConf, secondaryConf, err := r.reconcileConfigMap(ctx, cluster) + if err != nil { t.Fatalf("reconcileConfigMap: %v", err) } // Sanity: the pod IP must not have leaked into the rendered config. @@ -91,18 +78,18 @@ func TestConfigHashIndependentOfPrimaryPodIP(t *testing.T) { } for k, v := range cm.Data { if ip != "" && strings.Contains(v, ip) { - t.Fatalf("primary pod IP %s leaked into restart-scoped config %s:\n%s", ip, k, v) + t.Fatalf("primary pod IP %s leaked into rendered config %s:\n%s", ip, k, v) } } - return r.configHash(ctx, cluster) + return primaryConf + secondaryConf } - h1 := hashWithPrimaryIP("10.42.3.197") - h2 := hashWithPrimaryIP("10.42.9.42") // primary pod restarted onto a new IP + h1 := renderWithPrimaryIP("10.42.3.197") + h2 := renderWithPrimaryIP("10.42.9.42") // primary pod restarted onto a new IP if h1 == "" { - t.Fatal("config hash should not be empty") + t.Fatal("rendered config should not be empty") } if h1 != h2 { - t.Fatalf("config hash changed when only the primary pod IP changed — the v0.2.5 roll loop:\n%s\n%s", h1, h2) + t.Fatalf("rendered config changed when only the primary pod IP changed — the v0.2.5 loop:\n%s\n%s", h1, h2) } } diff --git a/internal/controller/reconfig_test.go b/internal/controller/reconfig_test.go new file mode 100644 index 0000000..ec39ee6 --- /dev/null +++ b/internal/controller/reconfig_test.go @@ -0,0 +1,190 @@ +package controller + +import ( + "context" + "errors" + "strings" + "testing" + + corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/equality" + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + + bindv1alpha1 "git.unkin.net/unkin/bind-operator/api/v1alpha1" + "git.unkin.net/unkin/bind-operator/internal/bind" +) + +func testScheme(t *testing.T) *runtime.Scheme { + t.Helper() + s := runtime.NewScheme() + if err := clientgoscheme.AddToScheme(s); err != nil { + t.Fatal(err) + } + if err := bindv1alpha1.AddToScheme(s); err != nil { + t.Fatal(err) + } + return s +} + +// fakeExec serves `cat` from mounted (per pod) and counts reconfigs. +type fakeExec struct { + mounted map[string]string + reconfigErr error + checkconf string + reconfigs map[string]int +} + +func (f *fakeExec) Exec(_ context.Context, _, pod string, cmd []string, _ string) (string, error) { + if len(cmd) == 2 && cmd[0] == bind.CheckconfBin { + return f.checkconf, errors.New("exit 1") + } + if len(cmd) != 3 || cmd[0] != "cat" || cmd[1] != bind.NamedConfRun || cmd[2] != bind.KeysConfPath { + return "", errors.New("unexpected command") + } + return f.mounted[pod], nil +} + +func (f *fakeExec) Reconfig(_ context.Context, _, pod string) error { + f.reconfigs[pod]++ + return f.reconfigErr +} + +func readyPod(cluster, name string) *corev1.Pod { + return &corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: "ns", Labels: commonLabels(cluster)}, + Status: corev1.PodStatus{Conditions: []corev1.PodCondition{{Type: corev1.PodReady, Status: corev1.ConditionTrue}}}, + } +} + +func TestPodTemplateIgnoresConfigButTracksPodSpec(t *testing.T) { + scheme := testScheme(t) + cluster := &bindv1alpha1.BindCluster{ + ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: "ns"}, + Spec: bindv1alpha1.BindClusterSpec{Replicas: 2}, + } + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(cluster).Build() + r := &BindClusterReconciler{Client: cl, Scheme: scheme} + ctx := context.Background() + + template := func() corev1.PodTemplateSpec { + t.Helper() + sts, err := r.reconcileStatefulSet(ctx, cluster) + if err != nil { + t.Fatal(err) + } + return sts.Spec.Template + } + setConfig := func(conf, keys string) { + t.Helper() + if err := r.upsertConfigMap(ctx, cluster, configMapName("c"), map[string]string{"named.conf.secondary": conf}); err != nil { + t.Fatal(err) + } + if err := r.upsertSecret(ctx, cluster, keysSecretName("c"), map[string][]byte{"keys.conf": []byte(keys)}); err != nil { + t.Fatal(err) + } + } + + setConfig("options { recursion yes; };", "") + base := template() + setConfig(`options { recursion no; }; zone "x" { type forward; forwarders { 1.1.1.1; }; };`, "key k {};") + if got := template(); !equality.Semantic.DeepEqual(base, got) { + t.Fatal("pod template changed on a rendered-config change; that restarts every pod") + } + + cluster.Spec.Image = "internetsystemsconsortium/bind9:9.21" + withImage := template() + if equality.Semantic.DeepEqual(base, withImage) { + t.Fatal("pod template must change when the image changes") + } + + cluster.Spec.Resources = corev1.ResourceRequirements{Limits: corev1.ResourceList{corev1.ResourceMemory: resource.MustParse("1Gi")}} + if equality.Semantic.DeepEqual(withImage, template()) { + t.Fatal("pod template must change when resources change") + } +} + +func TestReconfigPodsWaitsForMountThenReconfigsOnce(t *testing.T) { + scheme := testScheme(t) + cluster := &bindv1alpha1.BindCluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: "ns"}} + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(readyPod("c", "c-0"), readyPod("c", "c-1")).Build() + ex := &fakeExec{ + mounted: map[string]string{"c-0": "old-primary", "c-1": "old-secondary"}, + reconfigs: map[string]int{}, + } + r := &BindClusterReconciler{Client: cl, Scheme: scheme, Exec: ex} + ctx := context.Background() + const primary, secondary, keys = "new-primary;", "new-secondary;", "key k {};" + + if d := r.reconfigPods(ctx, cluster, primary, secondary, keys); d != requeueShort { + t.Fatalf("stale mounts should requeue short, got %v", d) + } + if len(ex.reconfigs) != 0 { + t.Fatalf("reconfig ran before the mount caught up: %v", ex.reconfigs) + } + if c := meta.FindStatusCondition(cluster.Status.Conditions, "ConfigApplied"); c == nil || c.Reason != "Propagating" { + t.Fatalf("want Propagating condition, got %+v", c) + } + + // Only the primary's mount has propagated. + ex.mounted["c-0"] = primary + keys + if d := r.reconfigPods(ctx, cluster, primary, secondary, keys); d != requeueShort { + t.Fatalf("secondary still stale should requeue, got %v", d) + } + if ex.reconfigs["c-0"] != 1 || ex.reconfigs["c-1"] != 0 { + t.Fatalf("want only c-0 reconfigured, got %v", ex.reconfigs) + } + + ex.mounted["c-1"] = secondary + keys + for range 3 { + if d := r.reconfigPods(ctx, cluster, primary, secondary, keys); d != 0 { + t.Fatalf("converged cluster should not requeue, got %v", d) + } + } + if ex.reconfigs["c-0"] != 1 || ex.reconfigs["c-1"] != 1 { + t.Fatalf("each pod must reconfig exactly once, got %v", ex.reconfigs) + } + if c := meta.FindStatusCondition(cluster.Status.Conditions, "ConfigApplied"); c == nil || c.Status != metav1.ConditionTrue { + t.Fatalf("want ConfigApplied=True, got %+v", c) + } + + // A pod that no longer exists drops out of the applied map. + if err := cl.Delete(ctx, readyPod("c", "c-1")); err != nil { + t.Fatal(err) + } + r.reconfigPods(ctx, cluster, primary, secondary, keys) + if _, ok := cluster.Status.AppliedConfig["c-1"]; ok { + t.Fatal("applied map kept a deleted pod") + } +} + +func TestReconfigPodsSurfacesFailure(t *testing.T) { + scheme := testScheme(t) + cluster := &bindv1alpha1.BindCluster{ObjectMeta: metav1.ObjectMeta{Name: "c", Namespace: "ns"}} + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(readyPod("c", "c-0")).Build() + ex := &fakeExec{ + mounted: map[string]string{"c-0": "bad;"}, + reconfigErr: errors.New("rndc: 'reconfig' failed: unexpected token"), + checkconf: "/run/named/named.conf:6: expected IP address near 'not-an-ip'", + reconfigs: map[string]int{}, + } + r := &BindClusterReconciler{Client: cl, Scheme: scheme, Exec: ex} + + if d := r.reconfigPods(context.Background(), cluster, "bad;", "", ""); d != requeueLong { + t.Fatalf("failure should requeue long, got %v", d) + } + c := meta.FindStatusCondition(cluster.Status.Conditions, "ConfigApplied") + if c == nil || c.Status != metav1.ConditionFalse || c.Reason != "ReconfigFailed" { + t.Fatalf("want ReconfigFailed condition, got %+v", c) + } + if !strings.Contains(c.Message, "named.conf:6: expected IP address") { + t.Fatalf("condition should carry the checkconf detail, got %q", c.Message) + } + if _, ok := cluster.Status.AppliedConfig["c-0"]; ok { + t.Fatal("failed reconfig must not be recorded as applied") + } +} diff --git a/internal/controller/util.go b/internal/controller/util.go index 12dccb8..a824cd0 100644 --- a/internal/controller/util.go +++ b/internal/controller/util.go @@ -25,16 +25,17 @@ func podReady(pod *corev1.Pod) bool { return false } -// entrypointScript selects the primary or secondary named.conf based on the -// pod's StatefulSet ordinal and launches named in the foreground. +// entrypointScript links the primary or secondary named.conf (by StatefulSet +// ordinal) and launches named in the foreground. A symlink, not a copy, so rndc +// reconfig reads the projected file kubelet keeps up to date. func entrypointScript() string { return fmt.Sprintf(`#!/bin/sh set -eu ORD="${HOSTNAME##*-}" if [ "$ORD" = "0" ]; then - cp %[1]s %[3]s + ln -sf %[1]s %[3]s else - cp %[2]s %[3]s + ln -sf %[2]s %[3]s fi mkdir -p %[4]s/zones %[4]s/catalog exec %[5]s -g -c %[3]s