Skip to content

config: Add versioned operator configuration through a CRD - #203

Open
HarshwardhanPatil07 wants to merge 14 commits into
bootc-dev:mainfrom
HarshwardhanPatil07:issue-92-configmap-config
Open

HarshwardhanPatil07 wants to merge 14 commits into
bootc-dev:mainfrom
HarshwardhanPatil07:issue-92-configmap-config

Conversation

@HarshwardhanPatil07

@HarshwardhanPatil07 HarshwardhanPatil07 commented Sep 29, 2026 •

Copy link
Copy Markdown
Member

Administrators currently need to edit operator workload arguments to change settings and preserve those edits during upgrades. Add an optional, administrator-owned, cluster-scoped BootcOperatorConfig using the versioned node.bootc.dev/v1alpha1 API. The resource may have any valid Kubernetes name, but the controller and daemon require at most one instance at startup.

The API exposes controller insecure-registry policy and image-tag resolution period, plus the daemon fallback status-poll period. Kubernetes validates types and positive whole-second periods. Both processes make one bounded, uncached list request at startup: zero instances use existing defaults, one supplies settings, and multiple instances or API errors stop startup with a contextual error. Explicit CLI flags take precedence, including false and fractional durations. Changes require a manual restart of the affected workload. Both service accounts receive only list permission for this resource.

The bink workflow now installs the CRD, waits for it, creates or updates its local-registry configuration, and only then deploys workloads. Install and Helm manifests include the CRD and permissions without creating or overwriting administrator configuration. Releases publish an optional, editable operator-config.yaml separately from install.yaml. Documentation covers precedence, restarts, revert and upgrade behavior, and Helm CRD upgrade ordering. Live updates, operator status reporting, and feature gates from #181 remain deferred.

Closes #92.

@alicefr

alicefr commented Sep 29, 2026

Copy link
Copy Markdown
Collaborator

I personally prefer a new CRD over a configmap. Mostly for 2 reasons, first it is versioned, secondly eventually we could use the crd status for reporting the operator status. The second point right now isn't used so the motivation isn't that strong.

I'd love also @cheesesashimi's opinion

@HarshwardhanPatil07 HarshwardhanPatil07 changed the title config: Support operator configuration through an optional ConfigMap config: Add versioned operator configuration through a CRD Sep 30, 2026
@HarshwardhanPatil07
HarshwardhanPatil07 force-pushed the issue-92-configmap-config branch 5 times, most recently from 16f3e66 to 19361d9 Compare September 30, 2026 11:24
@HarshwardhanPatil07

Copy link
Copy Markdown
Member Author

updated PTAL

Comment thread api/v1alpha1/bootcoperatorconfig_types.go Outdated
Comment thread internal/config/config.go Outdated
Comment thread Makefile Outdated
Comment thread test/e2e/config_test.go Outdated
Comment thread test/e2e/config_test.go Outdated
@alicefr

alicefr commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

We should also probably generate a cr as part of the release with some good defaults. This is at least how kubevirt is doing it

@HarshwardhanPatil07
HarshwardhanPatil07 force-pushed the issue-92-configmap-config branch 6 times, most recently from bfbeb93 to e2740b9 Compare October 6, 2026 07:48
@HarshwardhanPatil07

Copy link
Copy Markdown
Member Author

PTAL

Comment thread config/rbac/daemon_role.yaml
Comment thread Makefile Outdated
@alicefr

alicefr commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

@HarshwardhanPatil07 it looks great many thanks for doing this. Few last comment and this requires a rebase

@HarshwardhanPatil07
HarshwardhanPatil07 force-pushed the issue-92-configmap-config branch from e2740b9 to 7a4ebda Compare October 9, 2026 10:15
HarshwardhanPatil07 added a commit to HarshwardhanPatil07/bootc-operator that referenced this pull request Oct 12, 2026
Parallel E2E tests create daemon pods on other managed nodes. Restrict the membership lookup to its own node so unrelated pods do not cause the exactly-one assertion to time out.

Refs: bootc-dev#226
Refs: bootc-dev#203

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
alicefr pushed a commit that referenced this pull request Oct 12, 2026
Parallel E2E tests create daemon pods on other managed nodes. Restrict the membership lookup to its own node so unrelated pods do not cause the exactly-one assertion to time out.

Refs: #226
Refs: #203

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Comment thread docs/src/operations/configuration.md Outdated
Comment thread docs/src/operations/configuration.md Outdated
Comment thread docs/src/operations/configuration.md Outdated
Comment thread docs/src/operations/configuration.md Outdated
Comment thread docs/src/operations/configuration.md Outdated
Comment thread docs/src/operations/configuration.md Outdated
Comment thread docs/src/helm.md Outdated
@alicefr

alicefr commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

I'm still not quite sure about the helm manifest generation, I will give it a try tomorrow. IMO, helm should install the resources in the right order. If not, we might have a bug in our charts

@HarshwardhanPatil07
HarshwardhanPatil07 force-pushed the issue-92-configmap-config branch 2 times, most recently from 662d748 to 1287839 Compare October 19, 2026 11:01
@HarshwardhanPatil07

Copy link
Copy Markdown
Member Author

PTAL @alicefr

@alicefr

alicefr commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator

@HarshwardhanPatil07 if you don't mind, I'd like to test this with #232 . In this way, we can see if the CRDs correctly upgrade

Give administrators a typed cluster-scoped API for controller and daemon settings without editing workloads. Keep naming flexible; startup selection enforces that only one instance is used.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Prove API-server defaulting and rejection of invalid period, type, and field values before the startup path depends on the resource.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Preserve existing CLI behavior while allowing administrators to supply defaults through the resource. Explicit false values and fractional CLI durations continue to take precedence.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Read the administrator resource before informer caches exist. Treat missing CRDs, permission failures, API errors, and multiple instances as actionable startup errors while an absent instance keeps defaults.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Exercise absent, custom-named, updated, conflicting, and missing-CRD resources through envtest so the loader behavior is covered beyond mocked HTTP responses.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Make registry policy and tag resolution configurable on controller startup without changing explicit CLI overrides. Grant its service account only the list permission needed to select the singleton.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Let each daemon load the same administrator-owned resource at startup while preserving explicit CLI overrides. Give the daemon service account the list permission needed for the read.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Bink now installs the operator through Helm. Apply the local registry
configuration after the chart creates the CRD, then restart the controller
so it reads the settings and refreshes the development image.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
The release upgrade test temporarily removes current CRDs, which also deletes configuration. Snapshot and restore the optional resource during cleanup and restart workloads so later tests observe the original settings.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Give administrators a concrete example and document the single-instance rule, CLI precedence, manual restarts, and upgrade-safe ownership before they adopt the new API.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Connect Helm installation to optional administrator configuration and
clarify its deletion during default uninstall. Follow the CRD subchart
lifecycle introduced by bootc-dev#234, removing the obsolete generated CRD
directory so existing checkouts do not package duplicate definitions.

Refs: bootc-dev#92
Refs: bootc-dev#234

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Provide an editable example without putting an administrator-owned instance into install.yaml, so routine installs and upgrades cannot overwrite configuration.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Keep the downloadable example aligned with API defaults and ensure install.yaml ships the CRD without creating a configuration instance. Run the checks on every PR.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
Make the versioned API discoverable alongside each release. Validate both assets before publishing and direct administrators to check for an existing instance before applying the example.

Refs: bootc-dev#92

Assisted-by: AI
Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
@HarshwardhanPatil07

Copy link
Copy Markdown
Member Author

rebase

@HarshwardhanPatil07

Copy link
Copy Markdown
Member Author

@alicefr we have added the upgrade test here let's see if ci is happy here

Comment thread internal/config/config.go
if c := cfg.Spec.Controller; c != nil {
if c.AllowInsecureRegistry != nil {
if err := set(
"allow-insecure-registry",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we define some constant for the flag name and reuse it here and in the main.go. Same for the other flags

Comment thread internal/config/load.go
c, err := client.New(bootstrap, client.Options{Scheme: scheme, Mapper: mapper})
if err != nil {
return nil, fmt.Errorf("create client for BootcOperatorConfig: %w", err)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
defer httpClient.CloseIdleConnections()

To avoid leaking the connection

Comment thread hack/deploy-bink.sh
Comment on lines +1 to +30
#!/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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this logic really necessary? I mean the bootcconfig is defined by us and if there are more then one the operator should error out

Comment thread test/e2e/upgrade_test.go
// 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",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we get the default from the deployment and the daemonset definition?

@alicefr

alicefr commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

The PR looks good but I'm a bit afraid about the helm usage. Since, we install directly the crds and the operator in one helm operator, there is no room for the cluster admin to configure the bootcconfig at installation time. They always need to restart the operator and the daemon manually which isn't that great. @cheesesashimi any thought?

@alicefr

alicefr commented Oct 23, 2026

Copy link
Copy Markdown
Collaborator

What we could, it is to document a 3 steps installation procedure, if the cluster admin isn't happy with the default. Like:

  1. helm install bootc-operator-crds — installs CRDs
  2. Admin creates their BootcOperatorConfig CR
  3. helm install bootc-operator — installs the operator, which reads the CR at startup

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support a configmap-backed config dir for configuring the bootc-operator

2 participants