diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 81acda61..fa8b41f7 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -42,6 +42,9 @@ jobs: - name: Unit tests run: make unit + - name: Check release assets + run: make verify-release-manifest + - name: Vet run: make vet diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index c8556aca..a3b90bd5 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -83,6 +83,17 @@ jobs: echo "version=${VERSION}" >> "${GITHUB_OUTPUT}" echo "tag=v${VERSION}" >> "${GITHUB_OUTPUT}" + - name: Set up Go + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + cache: true # zizmor: ignore[cache-poisoning] + + - name: Build and check release manifests + env: + TAG: ${{ steps.version.outputs.tag }} + run: make verify-release-manifest IMG="${IMAGE}:${TAG}" + - name: Create and push tag env: TAG: ${{ steps.version.outputs.tag }} @@ -114,12 +125,6 @@ jobs: exit 1 fi - - name: Set up Go - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 - with: - go-version-file: go.mod - cache: true # zizmor: ignore[cache-poisoning] - - name: Build and push container image env: TAG: ${{ steps.version.outputs.tag }} @@ -130,11 +135,6 @@ jobs: podman login -u "${ACTOR}" -p "${GH_TOKEN}" ghcr.io podman push "${IMAGE}:${TAG}" - - name: Build install manifest - env: - TAG: ${{ steps.version.outputs.tag }} - run: make release-manifest IMG="${IMAGE}:${TAG}" - - name: Package and push Helm charts env: VERSION: ${{ steps.version.outputs.version }} @@ -169,6 +169,11 @@ jobs: printf 'kubectl apply -f https://github.com/%s/releases/download/%s/install.yaml\n' \ "${GITHUB_REPOSITORY}" "${TAG}" printf '```\n\n' + printf 'Optional configuration example: [operator-config.yaml](https://github.com/%s/releases/download/%s/operator-config.yaml). ' \ + "${GITHUB_REPOSITORY}" "${TAG}" + printf 'Before applying it, check for an existing instance; see the [configuration guide](https://github.com/%s/blob/%s/docs/src/operations/configuration.md).\n' \ + "${GITHUB_REPOSITORY}" "${TAG}" + printf '\n' printf '### Helm\n\n' printf '```bash\n' printf 'helm install bootc-operator oci://ghcr.io/%s/charts/bootc-operator --version %s \\\n' \ @@ -181,4 +186,4 @@ jobs: --notes-file notes.md \ --generate-notes \ --draft \ - install.yaml + install.yaml operator-config.yaml diff --git a/.gitignore b/.gitignore index 01dc05d8..15860136 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,8 @@ bin/ kubeconfig-* docs/book/ user-docs +/install.yaml +/operator-config.yaml chart/bootc-operator/crds/ chart/bootc-operator/charts/ chart/bootc-operator/Chart.lock diff --git a/Makefile b/Makefile index 09bf7638..4598a85e 100644 --- a/Makefile +++ b/Makefile @@ -166,11 +166,21 @@ buildimg: ## Build container image. $(CONTAINER_TOOL) build -t $(IMG) . .PHONY: release-manifest -release-manifest: kustomize yq ## Build install manifest (override IMG to set the image reference). +release-manifest: kustomize yq ## Build install manifest and optional configuration asset (override IMG). "$(KUSTOMIZE)" build config/default | \ "$(YQ)" '(select(.kind == "Deployment") | .spec.template.spec.containers[] | select(.name == "manager")).image = "$(IMG)"' | \ "$(YQ)" '(select(.kind == "DaemonSet") | .spec.template.spec.containers[] | select(.name == "daemon")).image = "$(IMG)"' \ > install.yaml + cp config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml operator-config.yaml + +.PHONY: verify-release-manifest +verify-release-manifest: release-manifest ## Check the generated release assets. + @cmp -s config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml operator-config.yaml || \ + { echo "operator-config.yaml differs from the validated example"; exit 1; } + @test "$$("$(YQ)" ea '[.] | map(select(.kind == "CustomResourceDefinition" and .metadata.name == "bootcoperatorconfigs.node.bootc.dev")) | length' install.yaml)" -eq 1 || \ + { echo "install.yaml must contain the BootcOperatorConfig CRD exactly once"; exit 1; } + @test "$$("$(YQ)" ea '[.] | map(select(.kind == "BootcOperatorConfig")) | length' install.yaml)" -eq 0 || \ + { echo "install.yaml must not create an administrator-owned BootcOperatorConfig"; exit 1; } .PHONY: build-update-image build-update-image: ## Build derived node images for update testing and push to bink registry. @@ -242,7 +252,9 @@ start-bink: seed-node-image ## Start a bink cluster (idempotent). kubectl --kubeconfig $(KUBECONFIG_BINK) wait --for=condition=Ready node/controller --timeout=5m .PHONY: deploy-bink -deploy-bink: start-bink build-update-image $(if $(RELEASED_OPERATOR_IMG),push-released-operator-image) helm ## Deploy to a bink cluster (requires: buildimg). +deploy-bink: start-bink helm ## Deploy to a bink cluster (requires: buildimg). + $(MAKE) build-update-image + $(if $(RELEASED_OPERATOR_IMG),$(MAKE) push-released-operator-image) podman push --tls-verify=false $(IMG) localhost:5000/bootc-operator-e2e:latest helm dependency build ./chart/bootc-operator img="$(IMG_BINK)" && \ @@ -251,10 +263,10 @@ deploy-bink: start-bink build-update-image $(if $(RELEASED_OPERATOR_IMG),push-re --create-namespace --namespace bootc-operator \ --set-string image.repository="$${img%:*}" \ --set-string image.tag="$${img##*:}" \ - --set-string image.pullPolicy=Always \ - --set-json 'controller.extraArgs=["--allow-insecure-registry","--tag-resolution-interval=10s"]' - kubectl --kubeconfig $(KUBECONFIG_BINK) -n bootc-operator rollout restart deployment/bootc-operator-controller - kubectl --kubeconfig $(KUBECONFIG_BINK) -n bootc-operator rollout status deployment/bootc-operator-controller --timeout=3m + --set-string image.pullPolicy=Always + KUBECONFIG="$(abspath $(KUBECONFIG_BINK))" YQ="$(YQ)" \ + KUBECTL="$(KUBECTL)" ./hack/deploy-bink.sh + .PHONY: gather-bink gather-bink: ## Gather diagnostic logs from the bink cluster. diff --git a/PROJECT b/PROJECT index 198623b1..5e64685f 100644 --- a/PROJECT +++ b/PROJECT @@ -26,4 +26,12 @@ resources: kind: BootcNode path: github.com/jlebon/bootc-operator/api/v1alpha1 version: v1alpha1 +- api: + crdVersion: v1 + namespaced: false + domain: bootc.dev + group: node + kind: BootcOperatorConfig + path: github.com/bootc-dev/bootc-operator/api/v1alpha1 + version: v1alpha1 version: "3" diff --git a/README.md b/README.md index 20c09df0..97bf1d4d 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,12 @@ kubectl apply -k https://github.com/bootc-dev/bootc-operator//config/default This creates the `bootc-operator` namespace and deploys the controller and daemon. The operator does nothing until you create a BootcNodePool. +To configure the controller and daemon with an optional +`BootcOperatorConfig` resource, see +[Configuring the operator](docs/src/operations/configuration.md). Releases +publish an editable `operator-config.yaml` example separately from +`install.yaml`. + > [!NOTE] > The operator namespace requires a [Pod Security Admission] exemption for the > `privileged` level. The daemon DaemonSet runs privileged to execute bootc diff --git a/api/v1alpha1/bootcoperatorconfig_types.go b/api/v1alpha1/bootcoperatorconfig_types.go new file mode 100644 index 00000000..6f89387f --- /dev/null +++ b/api/v1alpha1/bootcoperatorconfig_types.go @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + +const ( + // DefaultTagResolutionPeriodSeconds is the default interval between resolving image tags. + DefaultTagResolutionPeriodSeconds int32 = 300 + + // DefaultStatusPollPeriodSeconds is the default interval between fallback bootc status polls. + DefaultStatusPollPeriodSeconds int32 = 300 +) + +// OperatorControllerConfig configures the operator controller at startup. +type OperatorControllerConfig struct { + // allowInsecureRegistry allows falling back to HTTP when accessing image + // registries. Enable this only for trusted registries that do not support TLS. + // +optional + // +kubebuilder:default=false + AllowInsecureRegistry *bool `json:"allowInsecureRegistry,omitempty"` + + // tagResolutionPeriodSeconds is the interval in seconds between resolving + // image tags to digests. Defaults to 300 seconds. + // +optional + // +kubebuilder:default=300 + // +kubebuilder:validation:Minimum=1 + TagResolutionPeriodSeconds *int32 `json:"tagResolutionPeriodSeconds,omitempty"` +} + +// OperatorDaemonConfig configures each node daemon at startup. +type OperatorDaemonConfig struct { + // statusPollPeriodSeconds is the interval in seconds between bootc status + // polls when filesystem notifications are unavailable. Defaults to 300 seconds. + // +optional + // +kubebuilder:default=300 + // +kubebuilder:validation:Minimum=1 + StatusPollPeriodSeconds *int32 `json:"statusPollPeriodSeconds,omitempty"` +} + +// BootcOperatorConfigSpec defines operator settings consumed at process startup. +// Restart the affected controller or daemon pods after changing these settings. +type BootcOperatorConfigSpec struct { + // controller configures the controller. Omitted settings use their defaults. + // +optional + // +kubebuilder:default={} + Controller *OperatorControllerConfig `json:"controller,omitempty"` + + // daemon configures all node daemons. Omitted settings use their defaults. + // +optional + // +kubebuilder:default={} + Daemon *OperatorDaemonConfig `json:"daemon,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:resource:scope=Cluster + +// BootcOperatorConfig configures the controller and node daemons in a cluster. +// The optional resource is administrator-owned and may have any valid name. +// The operator rejects multiple configurations at startup. Changes take effect +// when the affected pods restart; explicit command-line flags take precedence. +type BootcOperatorConfig struct { + metav1.TypeMeta `json:",inline"` + + // metadata is standard object metadata. + // +optional + metav1.ObjectMeta `json:"metadata,omitzero"` + + // spec defines the configuration consumed when operator processes start. + // +required + Spec BootcOperatorConfigSpec `json:"spec"` +} + +// +kubebuilder:object:root=true + +// BootcOperatorConfigList contains a list of BootcOperatorConfig. +type BootcOperatorConfigList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitzero"` + Items []BootcOperatorConfig `json:"items"` +} diff --git a/api/v1alpha1/groupversion_info.go b/api/v1alpha1/groupversion_info.go index 2575b926..c5a1426d 100644 --- a/api/v1alpha1/groupversion_info.go +++ b/api/v1alpha1/groupversion_info.go @@ -30,6 +30,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &BootcNode{}, &BootcNodeList{}, &BootcNodePool{}, &BootcNodePoolList{}, + &BootcOperatorConfig{}, &BootcOperatorConfigList{}, ) metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index be71e825..fe75543b 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -249,6 +249,89 @@ func (in *BootcNodeStatus) DeepCopy() *BootcNodeStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BootcOperatorConfig) DeepCopyInto(out *BootcOperatorConfig) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BootcOperatorConfig. +func (in *BootcOperatorConfig) DeepCopy() *BootcOperatorConfig { + if in == nil { + return nil + } + out := new(BootcOperatorConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *BootcOperatorConfig) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BootcOperatorConfigList) DeepCopyInto(out *BootcOperatorConfigList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]BootcOperatorConfig, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BootcOperatorConfigList. +func (in *BootcOperatorConfigList) DeepCopy() *BootcOperatorConfigList { + if in == nil { + return nil + } + out := new(BootcOperatorConfigList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *BootcOperatorConfigList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BootcOperatorConfigSpec) DeepCopyInto(out *BootcOperatorConfigSpec) { + *out = *in + if in.Controller != nil { + in, out := &in.Controller, &out.Controller + *out = new(OperatorControllerConfig) + (*in).DeepCopyInto(*out) + } + if in.Daemon != nil { + in, out := &in.Daemon, &out.Daemon + *out = new(OperatorDaemonConfig) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BootcOperatorConfigSpec. +func (in *BootcOperatorConfigSpec) DeepCopy() *BootcOperatorConfigSpec { + if in == nil { + return nil + } + out := new(BootcOperatorConfigSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DisruptionSpec) DeepCopyInto(out *DisruptionSpec) { *out = *in @@ -298,6 +381,51 @@ func (in *ImageSpec) DeepCopy() *ImageSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OperatorControllerConfig) DeepCopyInto(out *OperatorControllerConfig) { + *out = *in + if in.AllowInsecureRegistry != nil { + in, out := &in.AllowInsecureRegistry, &out.AllowInsecureRegistry + *out = new(bool) + **out = **in + } + if in.TagResolutionPeriodSeconds != nil { + in, out := &in.TagResolutionPeriodSeconds, &out.TagResolutionPeriodSeconds + *out = new(int32) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OperatorControllerConfig. +func (in *OperatorControllerConfig) DeepCopy() *OperatorControllerConfig { + if in == nil { + return nil + } + out := new(OperatorControllerConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OperatorDaemonConfig) DeepCopyInto(out *OperatorDaemonConfig) { + *out = *in + if in.StatusPollPeriodSeconds != nil { + in, out := &in.StatusPollPeriodSeconds, &out.StatusPollPeriodSeconds + *out = new(int32) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OperatorDaemonConfig. +func (in *OperatorDaemonConfig) DeepCopy() *OperatorDaemonConfig { + if in == nil { + return nil + } + out := new(OperatorDaemonConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PullSecretRef) DeepCopyInto(out *PullSecretRef) { *out = *in diff --git a/cmd/controller/main.go b/cmd/controller/main.go index f15223d3..493c6a3c 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -17,6 +17,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" + "github.com/bootc-dev/bootc-operator/internal/config" "github.com/bootc-dev/bootc-operator/internal/controller" "github.com/bootc-dev/bootc-operator/internal/registry" "github.com/bootc-dev/bootc-operator/internal/version" @@ -58,7 +59,7 @@ func main() { flag.DurationVar( &tagResolutionInterval, "tag-resolution-interval", - 5*time.Minute, + time.Duration(bootcv1alpha1.DefaultTagResolutionPeriodSeconds)*time.Second, "How often to re-resolve tag-based image refs.", ) flag.BoolVar( @@ -75,6 +76,7 @@ func main() { } opts.BindFlags(flag.CommandLine) flag.Parse() + explicit := config.ExplicitFlags(flag.CommandLine) ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) @@ -88,7 +90,26 @@ func main() { version.GitCommit, ) - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + ctx := ctrl.SetupSignalHandler() + restConfig := ctrl.GetConfigOrDie() + operatorConfig, err := config.Load(ctx, restConfig, scheme) + if err != nil { + setupLog.Error(err, "Failed to load operator configuration") + os.Exit(1) + } + if err := config.ApplyToFlags(flag.CommandLine, operatorConfig); err != nil { + setupLog.Error(err, "Failed to apply operator configuration") + os.Exit(1) + } + config.LogSource( + setupLog, + operatorConfig, + explicit, + "allow-insecure-registry", + "tag-resolution-interval", + ) + + mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Scheme: scheme, HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, @@ -129,7 +150,7 @@ func main() { } setupLog.Info("Starting manager") - if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { + if err := mgr.Start(ctx); err != nil { setupLog.Error(err, "Failed to run manager") os.Exit(1) } diff --git a/cmd/daemon/main.go b/cmd/daemon/main.go index 829142f4..b6fed432 100644 --- a/cmd/daemon/main.go +++ b/cmd/daemon/main.go @@ -20,6 +20,7 @@ import ( bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" "github.com/bootc-dev/bootc-operator/internal/bootc" + "github.com/bootc-dev/bootc-operator/internal/config" "github.com/bootc-dev/bootc-operator/internal/daemon" "github.com/bootc-dev/bootc-operator/internal/version" ) @@ -39,7 +40,7 @@ func main() { flag.DurationVar( &pollInterval, "bootc-poll-interval", - 5*time.Minute, + time.Duration(bootcv1alpha1.DefaultStatusPollPeriodSeconds)*time.Second, "Interval for polling bootc status as a fallback to fsnotify", ) @@ -48,6 +49,7 @@ func main() { } opts.BindFlags(flag.CommandLine) flag.Parse() + explicit := config.ExplicitFlags(flag.CommandLine) ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) @@ -70,7 +72,20 @@ func main() { os.Exit(1) } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + ctx := ctrl.SetupSignalHandler() + restConfig := ctrl.GetConfigOrDie() + operatorConfig, err := config.Load(ctx, restConfig, scheme) + if err != nil { + setupLog.Error(err, "Failed to load operator configuration") + os.Exit(1) + } + if err := config.ApplyToFlags(flag.CommandLine, operatorConfig); err != nil { + setupLog.Error(err, "Failed to apply operator configuration") + os.Exit(1) + } + config.LogSource(setupLog, operatorConfig, explicit, "bootc-poll-interval") + + mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Scheme: scheme, // Only cache the BootcNode object for this node to avoid unnecessary watches. Cache: cache.Options{ @@ -115,7 +130,7 @@ func main() { } setupLog.Info("Starting daemon", "node", nodeName, "pollInterval", pollInterval) - if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { + if err := mgr.Start(ctx); err != nil { setupLog.Error(err, "Failed to run daemon") os.Exit(1) } diff --git a/config/bink/operator-config.yaml b/config/bink/operator-config.yaml new file mode 100644 index 00000000..48c2136c --- /dev/null +++ b/config/bink/operator-config.yaml @@ -0,0 +1,8 @@ +apiVersion: node.bootc.dev/v1alpha1 +kind: BootcOperatorConfig +metadata: + name: cluster +spec: + controller: + allowInsecureRegistry: true + tagResolutionPeriodSeconds: 10 diff --git a/config/crd/bases/node.bootc.dev_bootcoperatorconfigs.yaml b/config/crd/bases/node.bootc.dev_bootcoperatorconfigs.yaml new file mode 100644 index 00000000..d7900094 --- /dev/null +++ b/config/crd/bases/node.bootc.dev_bootcoperatorconfigs.yaml @@ -0,0 +1,86 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.20.1 + name: bootcoperatorconfigs.node.bootc.dev +spec: + group: node.bootc.dev + names: + kind: BootcOperatorConfig + listKind: BootcOperatorConfigList + plural: bootcoperatorconfigs + singular: bootcoperatorconfig + scope: Cluster + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: |- + BootcOperatorConfig configures the controller and node daemons in a cluster. + The optional resource is administrator-owned and may have any valid name. + The operator rejects multiple configurations at startup. Changes take effect + when the affected pods restart; explicit command-line flags take precedence. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: spec defines the configuration consumed when operator processes + start. + properties: + controller: + default: {} + description: controller configures the controller. Omitted settings + use their defaults. + properties: + allowInsecureRegistry: + default: false + description: |- + allowInsecureRegistry allows falling back to HTTP when accessing image + registries. Enable this only for trusted registries that do not support TLS. + type: boolean + tagResolutionPeriodSeconds: + default: 300 + description: |- + tagResolutionPeriodSeconds is the interval in seconds between resolving + image tags to digests. Defaults to 300 seconds. + format: int32 + minimum: 1 + type: integer + type: object + daemon: + default: {} + description: daemon configures all node daemons. Omitted settings + use their defaults. + properties: + statusPollPeriodSeconds: + default: 300 + description: |- + statusPollPeriodSeconds is the interval in seconds between bootc status + polls when filesystem notifications are unavailable. Defaults to 300 seconds. + format: int32 + minimum: 1 + type: integer + type: object + type: object + required: + - spec + type: object + served: true + storage: true diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 24041517..7230f693 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -1,4 +1,5 @@ resources: - bases/node.bootc.dev_bootcnodepools.yaml - bases/node.bootc.dev_bootcnodes.yaml +- bases/node.bootc.dev_bootcoperatorconfigs.yaml # +kubebuilder:scaffold:crdkustomizeresource diff --git a/config/rbac/daemon_role.yaml b/config/rbac/daemon_role.yaml index f026a8eb..16dcdd4c 100644 --- a/config/rbac/daemon_role.yaml +++ b/config/rbac/daemon_role.yaml @@ -6,6 +6,12 @@ metadata: app.kubernetes.io/managed-by: kustomize name: daemon-role rules: +- apiGroups: + - node.bootc.dev + resources: + - bootcoperatorconfigs + verbs: + - list - apiGroups: - "" resources: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index c25de146..fbb238f9 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -82,3 +82,9 @@ rules: - bootcnodes/status verbs: - get +- apiGroups: + - node.bootc.dev + resources: + - bootcoperatorconfigs + verbs: + - list diff --git a/config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml b/config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml new file mode 100644 index 00000000..c226059e --- /dev/null +++ b/config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml @@ -0,0 +1,10 @@ +apiVersion: node.bootc.dev/v1alpha1 +kind: BootcOperatorConfig +metadata: + name: cluster +spec: + controller: + allowInsecureRegistry: false + tagResolutionPeriodSeconds: 300 + daemon: + statusPollPeriodSeconds: 300 diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index fbfaf433..1e990ba4 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -7,4 +7,5 @@ - [Concepts](concepts.md) - [Helm Chart](helm.md) - [Operations](operations/index.md) + - [Configuring the operator](operations/configuration.md) - [Managing a pool](operations/pool.md) diff --git a/docs/src/helm.md b/docs/src/helm.md index 91a73e49..231b72a9 100644 --- a/docs/src/helm.md +++ b/docs/src/helm.md @@ -26,9 +26,10 @@ helm install bootc-operator oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc- --version 0.1.0 --create-namespace --namespace bootc-operator ``` -Alternatively, install from a local checkout: +Alternatively, generate and install the chart from a local checkout: ```bash +make helm helm dependency build ./chart/bootc-operator helm install bootc-operator ./chart/bootc-operator \ --create-namespace --namespace bootc-operator @@ -43,6 +44,10 @@ helm install bootc-operator oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc- --create-namespace --namespace bootc-operator --set crds.install=false ``` +The chart installs the configuration CRD and the operator's read permissions. +It does not create a `BootcOperatorConfig` instance. See +[Configuring the operator](operations/configuration.md) to add one. + By default the chart uses the image `ghcr.io/bootc-dev/bootc-operator:` where `appVersion` is defined in `Chart.yaml`. @@ -119,7 +124,9 @@ helm uninstall bootc-operator --namespace bootc-operator ``` > **Warning**: uninstalling the chart also removes the CRDs and all -> their instances cluster-wide. +> their instances cluster-wide, including administrator-owned +> `BootcOperatorConfig` settings. This applies when the CRD dependency is enabled +> (the default). Separately managed CRDs follow their own uninstall lifecycle. ## Regenerating the chart diff --git a/docs/src/operations/configuration.md b/docs/src/operations/configuration.md new file mode 100644 index 00000000..04fbde89 --- /dev/null +++ b/docs/src/operations/configuration.md @@ -0,0 +1,137 @@ +# Configuring the operator + +Configure the controller and daemon with a cluster-scoped +`BootcOperatorConfig`. Its typed `node.bootc.dev/v1alpha1` API keeps +administrator settings separate from installation manifests. Choose any valid +Kubernetes resource name; use at most one configuration in the cluster. + +For example: + +```yaml +apiVersion: node.bootc.dev/v1alpha1 +kind: BootcOperatorConfig +metadata: + name: cluster +spec: + controller: + tagResolutionPeriodSeconds: 60 + daemon: + statusPollPeriodSeconds: 60 +``` + +The operator installation includes the CRD and read permissions. The +configuration instance is optional, administrator-owned, and is not created or +overwritten by the default installation. With no instance, the controller and +daemon use their existing defaults and explicit command-line arguments. +`cluster` is only the example name; the resource has no namespace. If more +than one instance exists, both processes stop at startup until an +administrator removes the extras. + +## Supported settings + +| Field under spec | Component | Default | +| --- | --- | --- | +| `controller.allowInsecureRegistry` | Controller | `false` | +| `controller.tagResolutionPeriodSeconds` | Controller | `300` (5 minutes) | +| `daemon.statusPollPeriodSeconds` | Daemon | `300` (5 minutes) | + +Period values are positive whole seconds. Invalid types and zero or negative +periods are rejected by the API server. An empty `spec: {}` uses all defaults; +either component section may be omitted. +Use YAML booleans and integers directly, without string quotes. + +`allowInsecureRegistry` lets the controller's tag resolver fall back to HTTP +for registries without TLS. The bink development workflow enables it for its +local registry. Registry credentials continue to use each pool's +`pullSecretRef`. + +The daemon normally observes bootc status through filesystem notifications. +`statusPollPeriodSeconds` controls fallback polling when those notifications +are unavailable. + +## Applying configuration + +After installing or upgrading the operator, check for an existing configuration: + +```shell +kubectl get bootcoperatorconfigs +``` + +If none exists, edit and apply the sample: + +```shell +kubectl apply -f config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml +kubectl get bootcoperatorconfig cluster -o yaml +``` + +The sample contains the existing defaults (`false`, `300`, and `300`); edit it +to set the values you need before applying it. If a configuration already +exists, edit that resource instead of creating another. The sample name +`cluster` can be changed. +Reapplying `install.yaml` will not overwrite the configuration. + +Releases also provide a separate, optional `operator-config.yaml` asset with +the same default values. On a released installation, replace `vX.Y.Z` with +the installed release tag, then download and edit the file before applying it: + +```shell +curl -fL -o operator-config.yaml \ + https://github.com/bootc-dev/bootc-operator/releases/download/vX.Y.Z/operator-config.yaml +# Edit operator-config.yaml, then: +kubectl apply -f operator-config.yaml +``` + +Check for an existing instance first as shown above. The optional asset is +not part of `install.yaml`, so routine operator upgrades leave administrator +settings alone. + +The configuration is read once when each controller or daemon process starts. +Editing the resource does not restart pods or change running processes. +Restart the workload whose settings changed. The example above changes both: + +```shell +kubectl -n bootc-operator rollout restart deployment/bootc-operator-controller-manager +kubectl -n bootc-operator rollout status deployment/bootc-operator-controller-manager --timeout=3m +kubectl -n bootc-operator rollout restart daemonset/bootc-operator-daemon +kubectl -n bootc-operator rollout status daemonset/bootc-operator-daemon --timeout=3m +``` + +Use the workload names and namespace from your installation if customized. +Daemon pods run only on nodes selected by a pool. New pods read the current +configuration automatically. Restarting the controller alone does not +reconfigure existing daemon pods. + +## Precedence and startup failures + +For each supported setting, the value is selected in this order: + +1. An explicitly supplied command-line flag. +2. The corresponding configuration field. +3. The existing default when configuration is absent. + +An explicit `--allow-insecure-registry=false` overrides a resource that sets +`allowInsecureRegistry: true`. Existing duration flags keep their duration +syntax, including fractional seconds, independently of the whole-second API +fields. + +More than one configuration, a missing CRD, denied API access, or a connection +failure stops startup with an error. Remove extra configurations, install the +matching CRD and permissions, or restore API connectivity before restarting. +A running process keeps its loaded settings during a later API outage. + +## Reverting and upgrading + +Restore a field's previous value, or remove it to use its schema default, +then restart the affected workload. To remove all overrides from this resource: + +```shell +kubectl delete bootcoperatorconfig cluster +``` + +Replace `cluster` with the resource's actual name if you used a different one. +Then restart the affected controller and daemon pods as above. Deletion alone +does not change running processes. Explicit command-line flags still apply. + +Reapplying installation manifests during an ordinary upgrade preserves the +configuration resource. For Helm installation and removal behavior, see the +[Helm guide](../helm.md). diff --git a/hack/deploy-bink.sh b/hack/deploy-bink.sh new file mode 100755 index 00000000..f6550882 --- /dev/null +++ b/hack/deploy-bink.sh @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +# Configure the operator after make deploy-bink installs its Helm chart. +set -euo pipefail + +: "${KUBECONFIG:?must be set}" "${YQ:?must be set}" +: "${KUBECTL:=kubectl}" +cd "$(dirname "${BASH_SOURCE[0]}")/.." + +"$KUBECTL" --kubeconfig "$KUBECONFIG" wait --for=condition=Established \ + crd/bootcoperatorconfigs.node.bootc.dev --timeout=1m +if ! configs_json=$("$KUBECTL" --kubeconfig "$KUBECONFIG" get bootcoperatorconfigs -o json); then + printf 'Cannot list BootcOperatorConfig resources in the bink cluster.\n' >&2 + exit 1 +fi +if ! config_count=$(jq -r '.items | length' <<< "$configs_json") || \ + ! configs=$(jq -r '[.items[].metadata.name] | join(", ")' <<< "$configs_json"); then + printf 'Cannot read BootcOperatorConfig names from the bink cluster response.\n' >&2 + exit 1 +fi +if (( config_count > 1 )); then + printf 'Bink requires at most one BootcOperatorConfig; found: %s\n' "$configs" >&2 + exit 1 +fi +if (( config_count == 0 )); then + "$KUBECTL" --kubeconfig "$KUBECONFIG" create -f config/bink/operator-config.yaml +else + patch=$("$YQ" -o=json '{"spec": .spec}' config/bink/operator-config.yaml) + "$KUBECTL" --kubeconfig "$KUBECONFIG" patch bootcoperatorconfig "$configs" \ + --type=merge --patch "$patch" +fi +"$KUBECTL" --kubeconfig "$KUBECONFIG" -n bootc-operator rollout restart \ + deployment/bootc-operator-controller +"$KUBECTL" --kubeconfig "$KUBECONFIG" -n bootc-operator rollout status \ + deployment/bootc-operator-controller --timeout=3m diff --git a/hack/gather-logs.sh b/hack/gather-logs.sh index a580b12a..c87fe235 100755 --- a/hack/gather-logs.sh +++ b/hack/gather-logs.sh @@ -37,6 +37,7 @@ run "k-describe-pods.txt" kubectl describe pods -n bootc-operator run "k-get-deployment.yaml" kubectl get deployment -n bootc-operator -o yaml run "k-describe-bootcnodepools.txt" kubectl describe bootcnodepools run "k-describe-bootcnodes.txt" kubectl describe bootcnodes +run "k-get-operator-config.yaml" kubectl get bootcoperatorconfigs.node.bootc.dev -o yaml run "k-get-events.txt" kubectl get events -n bootc-operator --sort-by=.lastTimestamp # Pod logs diff --git a/hack/sync-helm.sh b/hack/sync-helm.sh index f55df976..39fc1fb7 100755 --- a/hack/sync-helm.sh +++ b/hack/sync-helm.sh @@ -16,6 +16,8 @@ command -v "$YQ" >/dev/null 2>&1 || { echo "error: yq required (run: make yq)" > # --- CRDs (separate chart) --- mkdir -p "$CRD_CHART_DIR/templates" cp "$CONFIG_DIR"/crd/bases/*.yaml "$CRD_CHART_DIR/templates/" +# Remove generated CRDs from the old layout to avoid packaging them twice. +rm -rf -- "$CHART_DIR/crds" echo "Synced CRDs" # --- RBAC --- diff --git a/internal/config/config.go b/internal/config/config.go new file mode 100644 index 00000000..48237f6d --- /dev/null +++ b/internal/config/config.go @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "flag" + "fmt" + "strconv" + "time" + + "github.com/go-logr/logr" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" +) + +// ExplicitFlags snapshots flags supplied on the command line, including false +// boolean values. Call it immediately after parsing, before applying configuration. +func ExplicitFlags(fs *flag.FlagSet) map[string]bool { + explicit := make(map[string]bool) + fs.Visit(func(f *flag.Flag) { explicit[f.Name] = true }) + return explicit +} + +// ApplyToFlags applies fields supported by this process to parsed flag values. +// It leaves explicitly supplied flags untouched, including false and fractional +// durations. Setting Flag.Value directly does not mark CR values as CLI flags. +// cfg must have been validated by Load. +func ApplyToFlags(fs *flag.FlagSet, cfg *bootcv1alpha1.BootcOperatorConfig) error { + if cfg == nil { + return nil + } + explicit := ExplicitFlags(fs) + set := func(name, value string) error { + f := fs.Lookup(name) + if f == nil || explicit[name] { + return nil // The other process may not register this flag. + } + if err := f.Value.Set(value); err != nil { + return fmt.Errorf("apply BootcOperatorConfig/%s to --%s: %w", cfg.Name, name, err) + } + return nil + } + if c := cfg.Spec.Controller; c != nil { + if c.AllowInsecureRegistry != nil { + if err := set( + "allow-insecure-registry", + strconv.FormatBool(*c.AllowInsecureRegistry), + ); err != nil { + return err + } + } + if c.TagResolutionPeriodSeconds != nil { + period := time.Duration(*c.TagResolutionPeriodSeconds) * time.Second + if err := set("tag-resolution-interval", period.String()); err != nil { + return err + } + } + } + if d := cfg.Spec.Daemon; d != nil && d.StatusPollPeriodSeconds != nil { + period := time.Duration(*d.StatusPollPeriodSeconds) * time.Second + if err := set("bootc-poll-interval", period.String()); err != nil { + return err + } + } + return nil +} + +// LogSource records the selected resource and CLI overrides without logging +// credentials or bootstrap flags. Supported flags are supplied by the component. +func LogSource( + log logr.Logger, + cfg *bootcv1alpha1.BootcOperatorConfig, + explicit map[string]bool, + flags ...string, +) { + overrides := make([]string, 0, len(flags)) + for _, name := range flags { + if explicit[name] { + overrides = append(overrides, name) + } + } + if cfg == nil { + log.Info("Loaded operator configuration", "source", "defaults", "cliOverrides", overrides) + return + } + log.Info("Loaded operator configuration", "source", "BootcOperatorConfig/"+cfg.Name, + "uid", cfg.UID, "generation", cfg.Generation, "cliOverrides", overrides) +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 00000000..940acde1 --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,139 @@ +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "errors" + "flag" + "math" + "testing" + "time" + + . "github.com/onsi/gomega" + "k8s.io/utils/ptr" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" +) + +func TestApplyToFlags(t *testing.T) { + for _, tt := range []struct { + name string + controller *bootcv1alpha1.OperatorControllerConfig + daemon *bootcv1alpha1.OperatorDaemonConfig + args []string + want bool + wantTag time.Duration + wantPoll time.Duration + }{ + {name: "absent instance", wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute}, + { + name: "empty sections", controller: &bootcv1alpha1.OperatorControllerConfig{}, + daemon: &bootcv1alpha1.OperatorDaemonConfig{}, wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute, + }, + { + name: "partial controller", controller: &bootcv1alpha1.OperatorControllerConfig{AllowInsecureRegistry: ptr.To(true)}, + want: true, wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute, + }, + { + name: "configured periods", + controller: &bootcv1alpha1.OperatorControllerConfig{TagResolutionPeriodSeconds: ptr.To(int32(10))}, + daemon: &bootcv1alpha1.OperatorDaemonConfig{StatusPollPeriodSeconds: ptr.To(int32(17))}, + wantTag: 10 * time.Second, wantPoll: 17 * time.Second, + }, + { + name: "maximum periods do not overflow", + controller: &bootcv1alpha1.OperatorControllerConfig{TagResolutionPeriodSeconds: ptr.To(int32(math.MaxInt32))}, + daemon: &bootcv1alpha1.OperatorDaemonConfig{StatusPollPeriodSeconds: ptr.To(int32(math.MaxInt32))}, + wantTag: time.Duration(math.MaxInt32) * time.Second, + wantPoll: time.Duration(math.MaxInt32) * time.Second, + }, + { + name: "explicit false and fractional durations override CR", + controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(true), TagResolutionPeriodSeconds: ptr.To(int32(10)), + }, + daemon: &bootcv1alpha1.OperatorDaemonConfig{StatusPollPeriodSeconds: ptr.To(int32(17))}, + args: []string{ + "--allow-insecure-registry=false", + "--tag-resolution-interval=1500ms", + "--bootc-poll-interval=500ms", + }, + wantTag: 1500 * time.Millisecond, + wantPoll: 500 * time.Millisecond, + }, + { + name: "explicit true overrides false", + controller: &bootcv1alpha1.OperatorControllerConfig{AllowInsecureRegistry: ptr.To(false)}, + args: []string{"--allow-insecure-registry"}, want: true, + wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute, + }, + { + name: "CLI retains zero and negative duration behavior", + controller: &bootcv1alpha1.OperatorControllerConfig{TagResolutionPeriodSeconds: ptr.To(int32(10))}, + daemon: &bootcv1alpha1.OperatorDaemonConfig{StatusPollPeriodSeconds: ptr.To(int32(17))}, + args: []string{"--tag-resolution-interval=0", "--bootc-poll-interval=-1s"}, + wantPoll: -time.Second, + }, + } { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + fs := flag.NewFlagSet(t.Name(), flag.ContinueOnError) + var allowInsecure bool + var tagInterval, pollInterval time.Duration + fs.BoolVar(&allowInsecure, "allow-insecure-registry", false, "") + fs.DurationVar(&tagInterval, "tag-resolution-interval", 5*time.Minute, "") + fs.DurationVar(&pollInterval, "bootc-poll-interval", 5*time.Minute, "") + g.Expect(fs.Parse(tt.args)).To(Succeed()) + explicit := ExplicitFlags(fs) + var cfg *bootcv1alpha1.BootcOperatorConfig + if tt.controller != nil || tt.daemon != nil { + cfg = &bootcv1alpha1.BootcOperatorConfig{ + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: tt.controller, + Daemon: tt.daemon, + }, + } + } + g.Expect(ApplyToFlags(fs, cfg)).To(Succeed()) + g.Expect(allowInsecure).To(Equal(tt.want)) + g.Expect(tagInterval).To(Equal(tt.wantTag)) + g.Expect(pollInterval).To(Equal(tt.wantPoll)) + g.Expect(ExplicitFlags(fs)). + To(Equal(explicit), "configuration must not mark flags as explicit") + }) + } +} + +func TestApplyToFlagsSkipsOtherComponent(t *testing.T) { + g := NewWithT(t) + fs := flag.NewFlagSet(t.Name(), flag.ContinueOnError) + var pollInterval time.Duration + fs.DurationVar(&pollInterval, "bootc-poll-interval", 5*time.Minute, "") + cfg := &bootcv1alpha1.BootcOperatorConfig{ + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(true), + }, + Daemon: &bootcv1alpha1.OperatorDaemonConfig{ + StatusPollPeriodSeconds: ptr.To(int32(17)), + }, + }, + } + g.Expect(ApplyToFlags(fs, cfg)).To(Succeed()) + g.Expect(pollInterval).To(Equal(17 * time.Second)) +} + +func TestApplyToFlagsReportsSetterError(t *testing.T) { + g := NewWithT(t) + fs := flag.NewFlagSet(t.Name(), flag.ContinueOnError) + fs.Func("allow-insecure-registry", "", func(string) error { return errors.New("rejected") }) + cfg := &bootcv1alpha1.BootcOperatorConfig{ + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(true), + }, + }, + } + g.Expect(ApplyToFlags(fs, cfg)). + To(MatchError(ContainSubstring("--allow-insecure-registry: rejected"))) +} diff --git a/internal/config/load.go b/internal/config/load.go new file mode 100644 index 00000000..a22d97a1 --- /dev/null +++ b/internal/config/load.go @@ -0,0 +1,105 @@ +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "context" + "fmt" + "sort" + "strings" + "time" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" +) + +// Bound the bootstrap read without imposing a timeout on the manager's watches. +const startupTimeout = 10 * time.Second + +// Load lists configurations directly from the API before caches start. An empty +// list permits defaults; installation, permission, and network failures do not. +func Load( + ctx context.Context, + restConfig *rest.Config, + scheme *runtime.Scheme, +) (*bootcv1alpha1.BootcOperatorConfig, error) { + return load(ctx, restConfig, scheme, startupTimeout) +} + +func load( + ctx context.Context, + restConfig *rest.Config, + scheme *runtime.Scheme, + timeout time.Duration, +) (*bootcv1alpha1.BootcOperatorConfig, error) { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + bootstrap := rest.CopyConfig(restConfig) + bootstrap.Timeout = timeout + // This client only lists our known resource, so API discovery is unnecessary. + gv := bootcv1alpha1.GroupVersion + mapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{gv}) + mapper.AddSpecific( + gv.WithKind("BootcOperatorConfig"), + gv.WithResource( + "bootcoperatorconfigs", + ), + gv.WithResource("bootcoperatorconfig"), + meta.RESTScopeRoot, + ) + c, err := client.New(bootstrap, client.Options{Scheme: scheme, Mapper: mapper}) + if err != nil { + return nil, fmt.Errorf("create client for BootcOperatorConfig: %w", err) + } + var configs bootcv1alpha1.BootcOperatorConfigList + if err := c.List(ctx, &configs); err != nil { + return nil, fmt.Errorf( + "list BootcOperatorConfig: ensure the bootcoperatorconfigs CRD is installed, "+ + "the API server is reachable, and this service account can list bootcoperatorconfigs: %w", + err, + ) + } + if len(configs.Items) == 0 { + return nil, nil + } + if len(configs.Items) > 1 { + names := make([]string, 0, len(configs.Items)) + for _, cfg := range configs.Items { + names = append(names, cfg.Name) + } + sort.Strings(names) + return nil, fmt.Errorf("found multiple BootcOperatorConfig resources (%s); keep only one", + strings.Join(names, ", ")) + } + cfg := &configs.Items[0] + if err := validate(cfg); err != nil { + return nil, err + } + return cfg, nil +} + +func validate(cfg *bootcv1alpha1.BootcOperatorConfig) error { + if cfg.Name == "" { + return fmt.Errorf("BootcOperatorConfig list returned an object without a name") + } + if c := cfg.Spec.Controller; c != nil && c.TagResolutionPeriodSeconds != nil && + *c.TagResolutionPeriodSeconds < 1 { + return fmt.Errorf( + "BootcOperatorConfig/%s spec.controller.tagResolutionPeriodSeconds must be at least 1", + cfg.Name, + ) + } + if d := cfg.Spec.Daemon; d != nil && d.StatusPollPeriodSeconds != nil && + *d.StatusPollPeriodSeconds < 1 { + return fmt.Errorf( + "BootcOperatorConfig/%s spec.daemon.statusPollPeriodSeconds must be at least 1", + cfg.Name, + ) + } + return nil +} diff --git a/internal/config/load_test.go b/internal/config/load_test.go new file mode 100644 index 00000000..0326070a --- /dev/null +++ b/internal/config/load_test.go @@ -0,0 +1,189 @@ +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "strings" + "testing" + "time" + + . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" +) + +const configPath = "/apis/node.bootc.dev/v1alpha1/bootcoperatorconfigs" + +func TestLoad(t *testing.T) { + resource := schema.GroupResource{Group: "node.bootc.dev", Resource: "bootcoperatorconfigs"} + for _, tt := range []struct { + name string + status *apierrors.StatusError + body string + code int + wantNil bool + wantError string + }{ + {name: "one custom-named object"}, + {name: "no objects", body: `{"items":[]}`, wantNil: true}, + { + name: "multiple objects", body: `{"items":[{"metadata":{"name":"z-config"}},` + + `{"metadata":{"name":"a-config"}}]}`, + wantError: "a-config, z-config); keep only one", + }, + { + name: "missing CRD", status: apierrors.NewNotFound(resource, ""), + wantError: "ensure the bootcoperatorconfigs CRD is installed", + }, + { + name: "plain HTTP 404", code: 404, body: "not found", + wantError: "ensure the bootcoperatorconfigs CRD is installed", + }, + { + name: "forbidden", status: apierrors.NewForbidden(resource, "", errors.New("access denied")), + wantError: "this service account can list bootcoperatorconfigs", + }, + { + name: "unauthorized", status: apierrors.NewUnauthorized("authentication failed"), + wantError: "authentication failed", + }, + {name: "malformed response", body: `{`, wantError: "unexpected end"}, + { + name: "zero daemon period", body: `{"items":[{"metadata":{"name":"custom-config"},` + + `"spec":{"daemon":{"statusPollPeriodSeconds":0}}}]}`, + wantError: "BootcOperatorConfig/custom-config spec.daemon.statusPollPeriodSeconds must be at least 1", + }, + { + name: "zero controller period", body: `{"items":[{"metadata":{"name":"custom-config"},` + + `"spec":{"controller":{"tagResolutionPeriodSeconds":0}}}]}`, + wantError: "BootcOperatorConfig/custom-config spec.controller.tagResolutionPeriodSeconds must be at least 1", + }, + { + name: "negative daemon period", body: `{"items":[{"metadata":{"name":"custom-config"},` + + `"spec":{"daemon":{"statusPollPeriodSeconds":-1}}}]}`, + wantError: "spec.daemon.statusPollPeriodSeconds must be at least 1", + }, + {name: "unnamed object", body: `{"items":[{"spec":{}}]}`, wantError: "object without a name"}, + } { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + var resourceRequests []string + wrapped := false + transport := roundTripFunc(func(req *http.Request) (*http.Response, error) { + g.Expect(req.Method).To(Equal(http.MethodGet)) + g.Expect(req.URL.Path).To(Equal(configPath), "list the resource without discovery") + g.Expect(req.URL.Query().Has("timeout")).To(BeTrue()) + g.Expect(req.URL.Query()).To(HaveLen(1), "list without a name or field selector") + resourceRequests = append(resourceRequests, req.URL.Path) + if tt.status != nil { + tt.status.ErrStatus.TypeMeta = metav1.TypeMeta{Kind: "Status", APIVersion: "v1"} + body, err := json.Marshal(tt.status.ErrStatus) + g.Expect(err).NotTo(HaveOccurred()) + return jsonResponse(int(tt.status.ErrStatus.Code), string(body)), nil + } + body := tt.body + if body == "" { + body = `{"items":[{"metadata":{"name":"custom-config","uid":"original",` + + `"generation":3},"spec":{"daemon":{"statusPollPeriodSeconds":17}}}]}` + } + code := tt.code + if code == 0 { + code = http.StatusOK + } + return jsonResponse(code, body), nil + }) + restConfig := &rest.Config{ + Host: "https://api.example.test", + Transport: transport, + Timeout: time.Minute, + } + restConfig.Wrap( + func(rt http.RoundTripper) http.RoundTripper { wrapped = true; return rt }, + ) + cfg, err := Load(context.Background(), restConfig, configScheme(t)) + g.Expect(restConfig.Timeout).To(Equal(time.Minute)) + g.Expect(wrapped).To(BeTrue(), "preserve existing transport wrappers") + g.Expect(resourceRequests).To(Equal([]string{configPath})) + if tt.wantError != "" { + g.Expect(err).To(MatchError(ContainSubstring(tt.wantError))) + g.Expect(cfg).To(BeNil()) + return + } + g.Expect(err).NotTo(HaveOccurred()) + if tt.wantNil { + g.Expect(cfg).To(BeNil()) + return + } + g.Expect(cfg.Name).To(Equal("custom-config")) + g.Expect(cfg.UID).To(BeEquivalentTo("original")) + g.Expect(cfg.Generation).To(Equal(int64(3))) + g.Expect(*cfg.Spec.Daemon.StatusPollPeriodSeconds).To(Equal(int32(17))) + }) + } +} + +func TestLoadRequestFailures(t *testing.T) { + for _, tt := range []struct{ failure, wantError string }{ + {"timeout", "deadline exceeded"}, + {"cancel", "context canceled"}, + {"connection", "connection refused"}, + } { + t.Run(tt.failure, func(t *testing.T) { + g := NewWithT(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + injected := false + transport := roundTripFunc(func(req *http.Request) (*http.Response, error) { + g.Expect(req.Method).To(Equal(http.MethodGet)) + g.Expect(req.URL.Path).To(Equal(configPath)) + injected = true + if tt.failure == "connection" { + return nil, errors.New("connection refused") + } + if tt.failure == "cancel" { + cancel() + } + <-req.Context().Done() + return nil, req.Context().Err() + }) + cfg, err := load( + ctx, + &rest.Config{Host: "https://api.example.test", Transport: transport}, + configScheme(t), + 20*time.Millisecond, + ) + g.Expect(injected).To(BeTrue()) + g.Expect(cfg).To(BeNil()) + g.Expect(err).To(MatchError(ContainSubstring(tt.wantError))) + }) + } +} + +func configScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + NewWithT(t).Expect(bootcv1alpha1.AddToScheme(scheme)).To(Succeed()) + return scheme +} + +func jsonResponse(code int, body string) *http.Response { + return &http.Response{ + StatusCode: code, + Header: http.Header{"Content-Type": {"application/json"}}, + Body: io.NopCloser(strings.NewReader(body)), + } +} + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (fn roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { return fn(req) } diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index 7d334476..34c3c787 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/internal/controller/bootcnodepool_controller.go @@ -78,6 +78,7 @@ type BootcNodePoolReconciler struct { } // +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcnodepools,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcoperatorconfigs,verbs=list // +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcnodepools/status,verbs=get;update;patch // +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcnodepools/finalizers,verbs=update // +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcnodes,verbs=get;list;watch;create;update;patch;delete diff --git a/internal/controller/operatorconfig_test.go b/internal/controller/operatorconfig_test.go new file mode 100644 index 00000000..9f3f6c71 --- /dev/null +++ b/internal/controller/operatorconfig_test.go @@ -0,0 +1,226 @@ +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "encoding/json" + "os" + "testing" + + . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + sigsyaml "sigs.k8s.io/yaml" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" + operatorconfig "github.com/bootc-dev/bootc-operator/internal/config" +) + +func TestBootcOperatorConfigDefaults(t *testing.T) { + const ( + tagDefault = bootcv1alpha1.DefaultTagResolutionPeriodSeconds + pollDefault = bootcv1alpha1.DefaultStatusPollPeriodSeconds + ) + for _, tc := range []struct { + name string + spec string + wantInsecure bool + wantTag int32 + wantPoll int32 + }{ + {"empty spec", `{}`, false, tagDefault, pollDefault}, + {"partial controller", `{"controller":{"allowInsecureRegistry":true}}`, true, tagDefault, pollDefault}, + {"partial daemon", `{"daemon":{"statusPollPeriodSeconds":17}}`, false, tagDefault, 17}, + {"explicit false", `{"controller":{"allowInsecureRegistry":false,"tagResolutionPeriodSeconds":10}}`, false, 10, pollDefault}, + {"null sections", `{"controller":null,"daemon":null}`, false, tagDefault, pollDefault}, + {"null fields", `{"controller":{"allowInsecureRegistry":null,"tagResolutionPeriodSeconds":null},"daemon":{"statusPollPeriodSeconds":null}}`, false, tagDefault, pollDefault}, + {"minimum periods", `{"controller":{"tagResolutionPeriodSeconds":1},"daemon":{"statusPollPeriodSeconds":1}}`, false, 1, 1}, + {"maximum periods", `{"controller":{"tagResolutionPeriodSeconds":2147483647},"daemon":{"statusPollPeriodSeconds":2147483647}}`, false, 2147483647, 2147483647}, + } { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + object := operatorConfigObject(t, "cluster", tc.spec) + g.Expect(k8sClient.Create(ctx, object)).To(Succeed()) + t.Cleanup(func() { + g.Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, object))).To(Succeed()) + }) + + var got bootcv1alpha1.BootcOperatorConfig + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(object), &got)).To(Succeed()) + g.Expect(got.Spec).To(Equal(bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(tc.wantInsecure), + TagResolutionPeriodSeconds: ptr.To(tc.wantTag), + }, + Daemon: &bootcv1alpha1.OperatorDaemonConfig{ + StatusPollPeriodSeconds: ptr.To(tc.wantPoll), + }, + })) + }) + } +} + +func TestBootcOperatorConfigReleaseExample(t *testing.T) { + g := NewWithT(t) + data, err := os.ReadFile("../../config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml") + g.Expect(err).NotTo(HaveOccurred()) + + var example bootcv1alpha1.BootcOperatorConfig + g.Expect(sigsyaml.UnmarshalStrict(data, &example)).To(Succeed()) + g.Expect(example.APIVersion).To(Equal(bootcv1alpha1.GroupVersion.String())) + g.Expect(example.Kind).To(Equal("BootcOperatorConfig")) + g.Expect(example.Name).To(Equal("cluster")) + g.Expect(example.Spec).To(Equal(bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(false), + TagResolutionPeriodSeconds: ptr.To(bootcv1alpha1.DefaultTagResolutionPeriodSeconds), + }, + Daemon: &bootcv1alpha1.OperatorDaemonConfig{ + StatusPollPeriodSeconds: ptr.To(bootcv1alpha1.DefaultStatusPollPeriodSeconds), + }, + })) + g.Expect(k8sClient.Create(context.Background(), &example, + client.DryRunAll, client.FieldValidation(metav1.FieldValidationStrict))).To(Succeed()) +} + +func TestBootcOperatorConfigValidation(t *testing.T) { + for _, tc := range []struct { + name string + spec string + field string + badRequest bool + }{ + {"missing spec", "", "spec", false}, + {"null spec", `null`, "spec", false}, + {"zero tag period", `{"controller":{"tagResolutionPeriodSeconds":0}}`, "spec.controller.tagResolutionPeriodSeconds", false}, + {"negative tag period", `{"controller":{"tagResolutionPeriodSeconds":-1}}`, "spec.controller.tagResolutionPeriodSeconds", false}, + {"zero poll period", `{"daemon":{"statusPollPeriodSeconds":0}}`, "spec.daemon.statusPollPeriodSeconds", false}, + {"negative poll period", `{"daemon":{"statusPollPeriodSeconds":-1}}`, "spec.daemon.statusPollPeriodSeconds", false}, + {"overflowing tag period", `{"controller":{"tagResolutionPeriodSeconds":2147483648}}`, "spec.controller.tagResolutionPeriodSeconds", false}, + {"overflowing poll period", `{"daemon":{"statusPollPeriodSeconds":2147483648}}`, "spec.daemon.statusPollPeriodSeconds", false}, + {"string period", `{"daemon":{"statusPollPeriodSeconds":"10s"}}`, "spec.daemon.statusPollPeriodSeconds", false}, + {"fractional period", `{"controller":{"tagResolutionPeriodSeconds":1.5}}`, "spec.controller.tagResolutionPeriodSeconds", false}, + {"string boolean", `{"controller":{"allowInsecureRegistry":"true"}}`, "spec.controller.allowInsecureRegistry", false}, + {"unknown field", `{"controller":{"tagResolutionPeriodSecond":10}}`, "spec.controller.tagResolutionPeriodSecond", true}, + } { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + object := operatorConfigObject(t, "cluster", tc.spec) + err := k8sClient.Create(context.Background(), object, + client.DryRunAll, client.FieldValidation(metav1.FieldValidationStrict)) + if tc.badRequest { + g.Expect(err).To(MatchError(apierrors.IsBadRequest, "IsBadRequest")) + } else { + g.Expect(err).To(MatchError(apierrors.IsInvalid, "IsInvalid")) + } + g.Expect(err.Error()).To(ContainSubstring(tc.field)) + }) + } +} + +func TestBootcOperatorConfigRoundTrip(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + loaded, err := operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(loaded).To(BeNil(), "an absent instance permits startup defaults") + config := &bootcv1alpha1.BootcOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "custom-config"}, + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(true), + TagResolutionPeriodSeconds: ptr.To(int32(10)), + }, + Daemon: &bootcv1alpha1.OperatorDaemonConfig{ + StatusPollPeriodSeconds: ptr.To(int32(17)), + }, + }, + } + wantSpec := *config.Spec.DeepCopy() + g.Expect(k8sClient.Create(ctx, config)).To(Succeed()) + t.Cleanup(func() { + g.Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, config))).To(Succeed()) + }) + got, err := operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).NotTo(BeNil()) + g.Expect(got.Name).To(Equal("custom-config")) + g.Expect(got.Spec).To(Equal(wantSpec)) + + startup := got + updated := got.DeepCopy() + updated.Spec.Controller.AllowInsecureRegistry = ptr.To(false) + updated.Spec.Daemon.StatusPollPeriodSeconds = ptr.To(int32(23)) + wantSpec = *updated.Spec.DeepCopy() + g.Expect(k8sClient.Update(ctx, updated)).To(Succeed()) + got, err = operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).NotTo(BeNil()) + g.Expect(got.Spec).To(Equal(wantSpec)) + g.Expect(got.UID).To(Equal(config.UID)) + g.Expect(got.Generation).To(Equal(config.Generation + 1)) + g.Expect(startup.Spec). + To(Equal(config.Spec), "previously loaded configuration remains a snapshot") + + g.Expect(k8sClient.Delete(ctx, got)).To(Succeed()) + loaded, err = operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(loaded).To(BeNil(), "deleting the instance restores defaults on the next load") +} + +func TestBootcOperatorConfigMultipleInstances(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + for _, name := range []string{"operator-one", "operator-two"} { + object := operatorConfigObject(t, name, `{}`) + g.Expect(k8sClient.Create(ctx, object)).To(Succeed(), "the API allows arbitrary names") + t.Cleanup(func() { + g.Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, object))).To(Succeed()) + }) + } + + loaded, err := operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(loaded).To(BeNil()) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(And( + ContainSubstring("operator-one"), + ContainSubstring("operator-two"), + )) +} + +func TestBootcOperatorConfigMissingCRD(t *testing.T) { + g := NewWithT(t) + // A separate API server keeps the missing-CRD case isolated from the suite. + env := &envtest.Environment{} + restConfig, err := env.Start() + g.Expect(err).NotTo(HaveOccurred()) + t.Cleanup(func() { g.Expect(env.Stop()).To(Succeed()) }) + + loaded, err := operatorconfig.Load(context.Background(), restConfig, k8sClient.Scheme()) + g.Expect(err).To(MatchError(apierrors.IsNotFound, "IsNotFound")) + g.Expect(err.Error()).To(ContainSubstring("ensure the bootcoperatorconfigs CRD is installed")) + g.Expect(loaded).To(BeNil()) +} + +// Use unstructured objects to exercise missing, null, and invalid fields that +// typed Go objects cannot represent on the wire. +func operatorConfigObject(t *testing.T, name, spec string) *unstructured.Unstructured { + t.Helper() + object := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": bootcv1alpha1.GroupVersion.String(), + "kind": "BootcOperatorConfig", + "metadata": map[string]any{"name": name}, + }} + if spec != "" { + var value any + NewWithT(t).Expect(json.Unmarshal([]byte(spec), &value)).To(Succeed()) + object.Object["spec"] = value + } + return object +} diff --git a/test/e2e/upgrade_test.go b/test/e2e/upgrade_test.go index 90172488..d78dbc53 100644 --- a/test/e2e/upgrade_test.go +++ b/test/e2e/upgrade_test.go @@ -5,6 +5,7 @@ package e2e import ( "context" "encoding/json" + "fmt" "os" "os/exec" "path/filepath" @@ -17,6 +18,7 @@ import ( corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" @@ -29,6 +31,7 @@ const helmReleaseName = "bootc-operator" var operatorCRDNames = []string{ "bootcnodepools.node.bootc.dev", "bootcnodes.node.bootc.dev", + "bootcoperatorconfigs.node.bootc.dev", } // TestOperatorUpgrade installs the released version of the operator @@ -66,21 +69,31 @@ func TestOperatorUpgrade(t *testing.T) { t.Logf("Current operator image: %s", currentImg) t.Logf("Released operator tag: %s, image: %s", releaseTag, releasedImg) - // Phase 1: Uninstall the current operator and re-install with - // the released image via Helm. - uninstallOperator(t, g, ctx, env) - helmInstallOperator(t, chartPath, releasedImg, extraArgs) - waitForOperatorReady(t, g, ctx, env.Client) - - t.Logf("Released operator %s installed via Helm", releaseTag) - + originalConfig, err := readOperatorConfig(ctx, env.Client) + g.Expect(err).NotTo(HaveOccurred()) + // Uninstall removes CRDs and their instances. Register recovery first. t.Cleanup(func() { t.Logf("Restoring operator to current version...") helmUpgradeOperator(t, chartPath, currentImg, extraArgs) + waitForOperatorConfigCRD(t, g, ctx, env.Client) + g.Expect(restoreOperatorConfig(ctx, env.Client, originalConfig)).To(Succeed(), + "restore administrator-owned operator configuration") + restartOperator(t) waitForOperatorReady(t, g, ctx, env.Client) t.Logf("Operator restored") }) + // Phase 1: Replace the current Helm release with the released image. + uninstallOperator(t, g, ctx, env) + // The released image predates BootcOperatorConfig and needs bink's + // registry settings through its command-line flags. + releasedArgs := append([]string{ + "--allow-insecure-registry", "--tag-resolution-interval=10s", + }, extraArgs...) + helmInstallOperator(t, chartPath, releasedImg, releasedArgs) + waitForOperatorReady(t, g, ctx, env.Client) + t.Logf("Released operator %s installed via Helm", releaseTag) + // Phase 2: Verify the released version works. nodeName := env.AddNode(t) @@ -91,9 +104,13 @@ func TestOperatorUpgrade(t *testing.T) { t.Logf("Node %q is Idle with released operator", nodeName) - // Phase 3: Upgrade via helm upgrade to the current version. + // Phase 3: Upgrade via Helm to the current version. helmUpgradeOperator(t, chartPath, currentImg, extraArgs) + waitForOperatorConfigCRD(t, g, ctx, env.Client) waitForOperatorReady(t, g, ctx, env.Client) + config, err := readOperatorConfig(ctx, env.Client) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(config).To(BeNil(), "installation must not create a configuration instance") t.Logf("Upgraded operator to current version via Helm") @@ -126,10 +143,8 @@ func TestOperatorUpgrade(t *testing.T) { Should(poolAllUpdated(1, env.NodeImageUpdateDigest())) } -// uninstallOperator removes the operator via Helm uninstall, then -// deletes CRDs (which Helm does not manage on uninstall). Custom -// resources are deleted first while the operator is still running so -// it can handle finalizer removal. +// uninstallOperator removes the Helm release and its CRDs. Custom +// resources are deleted first so the operator can handle finalizers. func uninstallOperator( t *testing.T, g Gomega, @@ -186,11 +201,12 @@ func helmInstallOperator(t *testing.T, chartPath, img string, extraArgs []string helmRun(t, args...) } -// helmUpgradeOperator upgrades the operator via helm upgrade. +// helmUpgradeOperator upgrades the operator, installing it if cleanup follows +// a failed release installation. func helmUpgradeOperator(t *testing.T, chartPath, img string, extraArgs []string) { t.Helper() args := []string{ - "upgrade", helmReleaseName, chartPath, + "upgrade", "--install", helmReleaseName, chartPath, "--namespace", testutil.OperatorNamespaceName, } args = append(args, helmImageArgs(t, img, extraArgs)...) @@ -284,6 +300,106 @@ func waitForOperatorReady( "expected operator deployment to be ready") } +// readOperatorConfig returns the optional administrator-owned instance. An +// ambiguous configuration should fail the upgrade instead of being discarded. +func readOperatorConfig( + ctx context.Context, + c client.Client, +) (*bootcv1alpha1.BootcOperatorConfig, error) { + var configs bootcv1alpha1.BootcOperatorConfigList + if err := c.List(ctx, &configs); err != nil { + return nil, fmt.Errorf("list operator configurations: %w", err) + } + switch len(configs.Items) { + case 0: + return nil, nil + case 1: + return configs.Items[0].DeepCopy(), nil + default: + return nil, fmt.Errorf( + "expected at most one operator configuration, found %d", + len(configs.Items), + ) + } +} + +// restoreOperatorConfig restores only administrator-owned fields after CRD +// deletion. A different instance name is an error rather than an implicit rename. +func restoreOperatorConfig( + ctx context.Context, + c client.Client, + original *bootcv1alpha1.BootcOperatorConfig, +) error { + current, err := readOperatorConfig(ctx, c) + if err != nil { + return err + } + if original == nil { + if current == nil { + return nil + } + return c.Delete(ctx, current) + } + if current != nil && current.Name != original.Name { + return fmt.Errorf( + "restore operator configuration %q: unexpected current instance %q", + original.Name, + current.Name, + ) + } + if current == nil { + restored := original.DeepCopy() + restored.ObjectMeta = metav1.ObjectMeta{ + Name: original.Name, + Labels: original.Labels, + Annotations: original.Annotations, + } + return c.Create(ctx, restored) + } + current.Spec = original.Spec + current.Labels = original.Labels + current.Annotations = original.Annotations + return c.Update(ctx, current) +} + +func waitForOperatorConfigCRD(t *testing.T, g Gomega, ctx context.Context, c client.Client) { + t.Helper() + g.Eventually(func() ([]apiextensionsv1.CustomResourceDefinitionCondition, error) { + var crd apiextensionsv1.CustomResourceDefinition + err := c.Get(ctx, client.ObjectKey{Name: "bootcoperatorconfigs.node.bootc.dev"}, &crd) + return crd.Status.Conditions, err + }).WithTimeout(time.Minute).Should(ContainElement(And( + HaveField("Type", apiextensionsv1.Established), + HaveField("Status", apiextensionsv1.ConditionTrue), + )), "waiting for the operator configuration API to be established") +} + +// Both components may have started with defaults before configuration was +// restored. Restart and await each workload so later tests see the saved values. +func restartOperator(t *testing.T) { + t.Helper() + workloads := []string{ + "deployment/" + operatorDeployKey().Name, + "daemonset/" + operatorDaemonSetKey().Name, + } + for _, workload := range workloads { + for _, command := range [][]string{ + {"rollout", "restart", workload}, + {"rollout", "status", workload, "--timeout=3m"}, + } { + ctx, cancel := context.WithTimeout(context.Background(), 4*time.Minute) + args := append([]string{ + "--kubeconfig", os.Getenv("KUBECONFIG"), "-n", testutil.OperatorNamespaceName, + }, command...) + out, err := exec.CommandContext(ctx, "kubectl", args...).CombinedOutput() + cancel() + if err != nil { + t.Fatalf("restart operator workload %s: %v\n%s", workload, err, out) + } + } + } +} + // operatorDeployKey returns the namespaced name of the operator Deployment. func operatorDeployKey() client.ObjectKey { return client.ObjectKey{