Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CC helm chart syntax #2863

Merged
merged 12 commits into from
Apr 18, 2024
Merged

Fix CC helm chart syntax #2863

merged 12 commits into from
Apr 18, 2024

Conversation

lisac
Copy link
Contributor

@lisac lisac commented Apr 17, 2024

What was the problem?

Associated tickets or Slack threads:

How does this fix it?1

This PR results in a valid helm chart being generated.

How to test this PR

  • verify that the helm upgrade step does not raise a parse error
  • Step 22 <-- considering the nature of this PR's changes, this may be unnecessary
  • verify that any debug code in the branch (eg dry runs, increased timeouts) is reverted

Footnotes

  1. Pull-Requests guidelines. If PR is significant, update Current Software State wiki page.

  2. To check if a PR will succeed in the SecRel workflow, test PRs in the SecRel pipeline.

@lisac lisac self-assigned this Apr 17, 2024
Copy link
Contributor

github-actions bot commented Apr 17, 2024

Test Results

151 tests  ±0   151 ✅ ±0   53s ⏱️ +6s
 46 suites ±0     0 💤 ±0 
 46 files   ±0     0 ❌ ±0 

Results for commit 095c76e. ± Comparison against base commit f434279.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Apr 17, 2024

JaCoCo Test Coverage

Overall Project 76.64%

There is no coverage information present for the Files changed

lisac and others added 11 commits April 17, 2024 15:59
… env variables?

why: trying to figure out whether introducing the postgres and alembic env variables is causing the deployment failure. these were introduced since the last successful build.
why: consistency with how the other env vars are pulled in, and it contains postgresUrl.

if we bring in just the postgresUrl, then for syntax purposes we might need to write out the key name POSTGRES_URL.
why: deployment is complaining about a nil pointer at this spot hard-coded the value found in helm/_shared/values.yaml#L8 .

error message:
> error calling include: [...] _helpers.tpl:15:26 executing "vro.annotations.pod" at <.Values.global.images.repo>: nil pointer evaluating interface {}.repo

https://github.com/department-of-veterans-affairs/abd-vro/blob/main/helm/_shared/values.yaml#L8
why: deployment is now complaining about nil pointer in this spot. something unusual about the `.Values.global.images.repo` lookup? 

> error calling include:  [...]_image.tpl:7:49: executing "vro.imageRegistryPath" at <.Values.global.images.repo>: nil pointer evaluating interface {}.repo
oops. in adding a `--debug`, i missed an important character.
the underlying issue is addressed in 056e689
group the vro.* values together, for readability
@lisac lisac changed the title WIP - debug CC helm chart deployment error Fix CC helm chart syntax Apr 18, 2024
@lisac lisac marked this pull request as ready for review April 18, 2024 19:41
@lisac lisac requested a review from a team as a code owner April 18, 2024 19:41
@lisac lisac requested review from tejans24 and chengjie8 April 18, 2024 19:41
Copy link
Contributor

@tejans24 tejans24 left a comment

Choose a reason for hiding this comment

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

nice, LGTM!

@lisac lisac merged commit 966faf0 into develop Apr 18, 2024
1 check passed
@lisac lisac deleted the lisac/cc-helm branch April 18, 2024 21:13
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