Skip to content

feat(bootstrap): manage local infrastructure with Argo CD - #731

Open
schrodit wants to merge 6 commits into
mainfrom
local-improvements
Open

feat(bootstrap): manage local infrastructure with Argo CD#731
schrodit wants to merge 6 commits into
mainfrom
local-improvements

Conversation

@schrodit

Copy link
Copy Markdown
Member

Summary

  • always bootstrap Argo CD for local installations and register Rook/Ceph and CloudNativePG as declarative Argo CD applications
  • wait until each application is healthy, synced, and compared against the requested target revision before continuing
  • add --ceph-device-filter and --ceph-device-path-filter options while retaining all-device selection as the default
  • mark the local codesphere-rbd storage class as the default storage class
  • make the installer hash optional, reuse shared package download/extraction helpers, and simplify app-of-apps setup
  • update the generated bootstrap-local command documentation and ignore local tmp/ artifacts

Testing

  • go test ./...
  • git diff --check

schrodit and others added 3 commits August 24, 2026 15:07
Install Rook and CloudNativePG through Argo CD applications and wait for the requested revisions to become healthy and synced before continuing. Add configurable Ceph device filters, make the local RBD storage class the default, streamline installer bundle preparation, and update the bootstrap-local CLI documentation.
Signed-off-by: schrodit <7979201+schrodit@users.noreply.github.com>
Document the exported bootstrap APIs, wrap cross-package errors with operation context, and apply the repository formatter to the changed files.

@OliverTrautvetter OliverTrautvetter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some questions and ideas to improve it

Comment thread internal/bootstrap/local/postgres.go Outdated
CreateNamespace: true,
return b.installHelmApplication(helmApplicationConfig{
Name: cnpgReleaseName, Chart: "cloudnative-pg", RepoURL: cnpgRepoURL,
TargetRevision: "*", Namespace: codesphereNamespace,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For rook we have
rookVersion = "v1.19.6"
but here we use the latest version. So Argo CD will silently upgrade the cloudnative-pg operator to whatever latest is.

Do we want that?

in postgres.go we have it pinned
cnpgDatabaseVersion = "15.14"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I guess even better would be to use the version from the bom

Comment on lines +142 to +145
return b.installHelmApplication(helmApplicationConfig{
Name: rookReleaseName, Chart: "rook-ceph", RepoURL: rookRepoURL,
TargetRevision: rookVersion, Namespace: rookNamespace, Values: helmValues,
})

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What happens when this runs against a cluster where Rook/CNPG were previously installed via direct helm upgrade?

Or do we just assume it is only used on fresh clusters?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

doesn't matter argo just takes over

cephBlockPoolName = "codesphere-rbd"
cephStorageClassName = "codesphere-rbd"
cephRBDProvisionerName = "rook-ceph.rbd.csi.ceph.com"
defaultStorageClassAnnotation = "storageclass.kubernetes.io/is-default-class"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what if something else is already default class?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

good point. will add a check for that and only set a default class if there is not another one

Comment on lines +43 to +48
if err := kubeClient.Get(ctx, client.ObjectKey{Name: name, Namespace: DefaultNamespace}, app); err != nil {
logf("Waiting for ArgoCD Application %q: failed to read status: %v", name, err)
return false, nil
}

lastHealth, lastSync = string(app.Status.Health.Status), string(app.Status.Sync.Status)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Would a non NotFound error not also trigger this 30 minute timeout? So for example a 403 Forbidden?

Comment thread internal/bootstrap/local/argocd_app.go Outdated
}

desired := &argov1alpha1.Application{
ObjectMeta: metav1.ObjectMeta{Name: cfg.Name, Namespace: "argocd"},

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit, this could use DefaultNamespace instead of "argocd"

Prepare the installer bundle before bootstrap dependencies and pin the CloudNativePG operator chart to the version from the installer BOM. Preserve an existing default storage class and fail fast on non-retryable Argo CD API errors.
@schrodit

Copy link
Copy Markdown
Member Author

Implemented the review feedback in commit b44209a:

  • prepare and retain the installer bundle before all other bootstrap steps so its BOM is available up front
  • use bom.Config.GetChart("postgres-operator") to pin the CloudNativePG operator chart to the installer BOM version instead of tracking *
  • keep an existing default StorageClass authoritative; the Rook class is only marked default when no other default exists (including the legacy annotation)
  • let Argo CD continue taking over existing Helm-managed Rook/CNPG releases
  • fail fast on non-NotFound errors while waiting for Argo CD Applications
  • use argocd.DefaultNamespace for locally managed Applications

Verification:

  • go test ./...
  • focused tests for the changed packages
  • golangci-lint --new-from-merge-base=origin/main

Parse the installer BOM during bundle preparation and pass the resulting config to CNPG and pc-apps consumers. Keep BOM file handling at orchestration boundaries instead of reparsing paths in downstream installers.
@schrodit

Copy link
Copy Markdown
Member Author

Follow-up implemented in c4eea06 based on the latest feedback:

  • the local bootstrap now parses deps/bom.json once in PrepareInstaller and stores the resulting *bom.Config
  • CNPG and pc-apps receive and reuse that parsed BOM instead of receiving a path and parsing it again
  • the regular dependency-install flow likewise parses once at its orchestration boundary before passing the BOM to pc-apps

Verification remains green: go test ./... and changed-code golangci-lint.

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.

2 participants