Uploaded image for project: 'FlightPath'
  1. FlightPath
  2. FLPATH-3221

Remove duplicate bucket creation job from Helm chart

XMLWordPrintable

    • False
    • Hide

      None

      Show
      None
    • False

      Summary

      Bucket creation happens in two places:

      • The install script (scripts/install-helm-chart.sh) creates S3 buckets using a one-shot kubectl run pod before helm install
      • The Helm chart creates the same buckets via a post-install,post-upgrade hook job (templates/infrastructure/storage/job-minio-buckets.yaml) after helm install

      Both create the same 3 buckets: insights-upload-perma, koku-bucket, ros-data.

      Goal

      Remove the Helm hook job and keep the install script as the sole bucket creation mechanism. The install script runs before Helm and validates S3 connectivity, which provides earlier failure detection.

      Additionally, fix three bucket naming bugs discovered during triage.

      Bugs Found During Triage

      Bug 1: REQUESTED_BUCKET defaults to wrong bucket

      In _helpers-koku.tpl line 340, when useExternalOBC=true and odf.bucket is unset, REQUESTED_BUCKET defaults to "ros-data" instead of "koku-bucket".

      REQUESTED_BUCKET is Koku's cost data bucket (confirmed in koku settings.py — it feeds S3_BUCKET_NAME via EnvConfigurator). Defaulting to "ros-data" would cause Koku to read/write cost reports to the ROS bucket.

      Current code:

      value: {{ .Values.odf.bucket | default "ros-data" | quote }}
      

      Should be:

      value: {{ include "cost-onprem.storage.kokuBucket" . | quote }}
      

      Confidence the code is wrong: HIGH. The default "ros-data" is clearly incorrect — REQUESTED_BUCKET feeds S3_BUCKET_NAME in Koku for cost report processing, not ROS data. The else branch on the very next line uses costManagement.storage.bucketName (default "koku-bucket"), confirming the intent.

      Confidence it causes runtime issues: LOW. The useExternalOBC branch only triggers when someone explicitly sets odf.useExternalOBC=true. The install script always sets both odf.bucket and costManagement.storage.bucketName in that path, so the bad default never triggers in practice. It would only manifest if someone manually crafts --set odf.useExternalOBC=true without also setting odf.bucket.

      Action: Fix it. Low risk, clearly wrong code, trivial to correct. Replace the entire conditional with the new helper.

      Bug 2: odf.bucket (singular) is undefined in values.yaml

      odf.bucket (singular) is referenced in two template helpers but never defined in values.yaml. Only odf.buckets (plural, a list) exists. Since the odf section always exists in values.yaml, .Values.odf.bucket resolves to nil/empty string.

      Affected templates:

      • _helpers.tpl:543cost-onprem.storage.bucket helper (for INGRESS_STAGEBUCKET)
      • _helpers-koku.tpl:340REQUESTED_BUCKET env var

      Confidence the key is undefined: HIGH. Only odf.buckets (plural) exists in values.yaml.

      Confidence it causes runtime issues: MEDIUM. The helper always enters the if .Values.odf branch (because the odf section exists), and .Values.odf.bucket resolves to nil/empty in Go templates, making INGRESS_STAGEBUCKET="". However, the system works in practice — uploads succeed and E2E tests pass — suggesting insights-ingress-go may have an internal default when the env var is empty.

      Action: Fix it. The template code is objectively wrong (reads a non-existent key while the correct value sits unused at ingress.storage.bucket). Even if the ingress has an internal default, the template should produce the correct value.

      Bug 3: ingress.storage.bucket is defined but never consumed

      ingress.storage.bucket is properly defined at values.yaml:706 as "insights-upload-perma" but the cost-onprem.storage.bucket helper reads odf.bucket (nil) instead of ingress.storage.bucket.

      Confidence this is a code inconsistency: HIGH. The value is well-commented as INGRESS_STAGEBUCKET but the helper reads the wrong key.

      Confidence it causes runtime issues: Same as Bug 2 — they are two sides of the same coin. Bug 2 is the symptom (reading a nil key), Bug 3 is the root cause (reading the wrong key).

      Action: Fix it. Change the helper from odf.bucket to ingress.storage.bucket. Zero risk because the value is already correctly defined.

      Risk Assessment

      Change Risk Rationale
      Fix cost-onprem.storage.bucket helper Very low Points at a value that already exists with the correct string
      New kokuBucket / rosBucket helpers Very low Thin wrappers over values already in use
      Remove useExternalOBC conditional from REQUESTED_BUCKET Low External OBC path already sets costManagement.storage.bucketName, so the new helper produces the same result
      Update install script external OBC path Low Changing -set odf.bucket= to -set ingress.storage.bucket= — just wiring to the correct value
      Delete bucket job Very low Install script already creates all buckets before Helm runs

      The main risk area is the useExternalOBC flow, since it is harder to test (requires a real Ceph RGW setup). But the logic is straightforward — the install script already sets costManagement.storage.bucketName in that path, so the new helper will resolve correctly.

      Changes

      • Delete cost-onprem/templates/infrastructure/storage/job-minio-buckets.yaml (the Helm hook job)
      • Remove global.initContainers.minioMc image config from values.yaml (only used by the deleted job)
      • Remove odf.buckets list from values.yaml (only used by the deleted job)
      • Add rosBucketName: "ros-data" to costManagement.storage in values.yaml (was only a template default, never explicitly declared)
      • Create standardized bucket helpers in _helpers.tpl: cost-onprem.storage.kokuBucket and cost-onprem.storage.rosBucket
      • Fix cost-onprem.storage.bucket helper to read ingress.storage.bucket instead of odf.bucket
      • Fix REQUESTED_BUCKET and REQUESTED_ROS_BUCKET in _helpers-koku.tpl to use the new standardized helpers
      • Remove old cost-onprem.koku.s3.rosBucket helper (replaced by cost-onprem.storage.rosBucket)
      • Fix useExternalOBC path in install-helm-chart.sh to set ingress.storage.bucket instead of odf.bucket
      • Update create_s3_buckets() to read bucket names from values.yaml via yq instead of hardcoding
      • Consolidate duplicated FQDN namespace parsing in install-helm-chart.sh into a reusable helper function (per masayag review on PR #88)
      • Remove duplicated ODF NooBaa detection logic in create_s3_buckets()
      • Add cleanup subcommand to deploy-minio-test.sh: ./scripts/deploy-minio-test.sh cost-onprem cleanup (per masayag review on PR #88)
      • Update docs/development/ocp-dev-setup-minio.md cleanup section and remove references to the Helm bucket job

      Affected Files

      • cost-onprem/templates/infrastructure/storage/job-minio-buckets.yaml (delete)
      • cost-onprem/templates/_helpers.tpl (add kokuBucket/rosBucket helpers, fix storage.bucket)
      • cost-onprem/templates/_helpers-koku.tpl (use new helpers, remove old rosBucket)
      • cost-onprem/values.yaml (remove minioMc/odf.buckets, add rosBucketName)
      • scripts/install-helm-chart.sh (FQDN parsing, dynamic buckets, fix external OBC path)
      • scripts/deploy-minio-test.sh (add cleanup subcommand)
      • docs/development/ocp-dev-setup-minio.md (update cleanup section)

      Context

      Identified during the MinIO port mismatch fix (FLPATH-3220). PR #88 review feedback from masayag incorporated. Bucket naming bugs found by triaging Koku source code (koku/settings.py, koku/configurator.py) and ros-ocp-backend to understand how REQUESTED_BUCKET and REQUESTED_ROS_BUCKET are consumed.

              jgil@redhat.com Jordi Gil
              jgil@redhat.com Jordi Gil
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

                Created:
                Updated:
                Resolved: