fix: ensure flux check returns non-zero exit code on failure
#5267
Conversation
pkg/status/status.go
Outdated
| for _, id := range identifiers { | ||
| rs := coll.ResourceStatuses[id] | ||
| switch rs.Status { | ||
| case status.CurrentStatus: | ||
| sc.logger.Successf("%s: %s ready", rs.Identifier.Name, strings.ToLower(rs.Identifier.GroupKind.Kind)) | ||
| case status.NotFoundStatus: | ||
| sc.logger.Failuref("%s: %s not found", rs.Identifier.Name, strings.ToLower(rs.Identifier.GroupKind.Kind)) | ||
| errMsg := fmt.Sprintf("%s: %s not found", rs.Identifier.Name, strings.ToLower(rs.Identifier.GroupKind.Kind)) | ||
| sc.logger.Failuref(errMsg) |
There was a problem hiding this comment.
I'm not sure what's gonna happen now that we both log and return the error, would we see the error printed twice? Can you please test that? Maybe we need to remove the log.
There was a problem hiding this comment.
StatusChecker.Assess is used in three places with inconsistent error handling:
-
check.go
Lines 215 to 217 in f0fecf7
- Does not use the error
-
install.go
Lines 279 to 281 in f0fecf7
- Uses the error
-
bootstrap_plain_git.goflux2/pkg/bootstrap/bootstrap_plain_git.go
Lines 492 to 494 in f0fecf7
- Uses the error
This means either:
- At least one of the locations logs the error twice, or
check.gois missing necessary logging or needs refactoring
There was a problem hiding this comment.
Yeah maybe we need some refactoring here... We're busy preparing for KubeCon EU next week, can you please ping us again after KubeCon? Thanks 🙏
Signed-off-by: h3nryc0ding <[email protected]>
|
Hey @matheuscscp 👋 I've done some refactoring to separate business and presentation logic. Let me know what you think. Some examples logs:
$ flux check
► Checking flux pre-requisites
✔ flux 0.0.0-dev.0 is a development build
► Checking kubernetes pre-requisites
✔ kubernetes 1.32.0>=1.30.0-0
► Checking flux installation status
✔ distribution: flux-v2.4.0
✔ bootstrapped: true
► Checking flux controllers
✔ ghcr.io/fluxcd/helm-controller:v1.1.0
✔ ghcr.io/fluxcd/kustomize-controller:v1.4.0
✔ ghcr.io/fluxcd/notification-controller:v1.4.0
✔ ghcr.io/fluxcd/source-controller:v1.4.1
► Checking flux crds
✔ alerts.notification.toolkit.fluxcd.io/v1beta3
✔ buckets.source.toolkit.fluxcd.io/v1
✔ gitrepositories.source.toolkit.fluxcd.io/v1
✔ helmcharts.source.toolkit.fluxcd.io/v1
✔ helmreleases.helm.toolkit.fluxcd.io/v2
✔ helmrepositories.source.toolkit.fluxcd.io/v1
✔ kustomizations.kustomize.toolkit.fluxcd.io/v1
✔ ocirepositories.source.toolkit.fluxcd.io/v1beta2
✔ providers.notification.toolkit.fluxcd.io/v1beta3
✔ receivers.notification.toolkit.fluxcd.io/v1
► All checks passed
$ flux check --pre
► Checking flux pre-requisites
✔ flux 0.0.0-dev.0 is a development build
► Checking kubernetes pre-requisites
✔ kubernetes 1.32.0>=1.30.0-0
► All pre-installation checks passed
$ flux check
► Checking flux pre-requisites
✔ flux 0.0.0-dev.0 is a development build
► Checking kubernetes pre-requisites
✔ kubernetes 1.30.7+orb1>=1.30.0-0
► Checking flux installation status
✗ error getting cluster info: customresourcedefinitions.apiextensions.k8s.io "gitrepositories.source.toolkit.fluxcd.io" not found
► Checking flux controllers
✗ no controllers found in the 'flux-system' namespace with the label selector 'app.kubernetes.io/part-of=flux'
► Checking flux crds
✗ no crds found with the label selector 'app.kubernetes.io/part-of=flux'
✗ checks failed
► Checking flux pre-requisites
✔ flux 0.0.0-dev.0 is a development build
► Checking kubernetes pre-requisites
✔ kubernetes 1.30.7+orb1>=1.30.0-0
► All pre-installation checks passed |
Signed-off-by: h3nryc0ding <[email protected]>
Signed-off-by: h3nryc0ding <[email protected]>
|
Hi @h3nryc0ding, was the 4th output a mistake on copy-paste?
|
| if !kubernetesCheck(cfg, kubernetesConstraints) { | ||
| checkFailed = true | ||
| if !runPreChecks(ctx, cfg) { | ||
| return errors.New("pre-installation checks failed") |
There was a problem hiding this comment.
If --pre was not passed, we should continue with the other checks and not exit here.
Fixes #5220 by returning a non-zero exist code for failed checks.