Skip to content

Chronicle official release chart cleanup#670

Merged
bschwedler merged 71 commits intomainfrom
chronicle/chart-cleanup
Jun 9, 2025
Merged

Chronicle official release chart cleanup#670
bschwedler merged 71 commits intomainfrom
chronicle/chart-cleanup

Conversation

@ianpittwood
Copy link
Contributor

  • Improvements for chart annotations.
  • Values changes.
    • Allow name and namespace overrides in chart values.
    • Add common labels and annotations values to apply to all resources.
    • Moves default tag source to appVersion, image.tag changed to a blank override.
    • Separated an image.registry value from the image.repository value.
    • Improve documentation of values.yaml and add a values.schema.json definition for input validation.
    • An S3 bucket must now be specified in S3 Storage backend is enabled.
  • Changes to chart behavior.
    • Resource names are now applied dynamically based on the release name.
    • Additional default recommended Kubernetes labels have been applied to all resources.
    • Storage configuration is now validated and requires at least one of local or s3 storage be enabled.
    • extraSecretMounts can now be specified to mount additional secrets, such as certificates, into the pod.
    • Storage class can now be overridden on the pod's volume claim template.
    • Selector labels definitions between pod and service are now merged into a single definition. Removed the ability to override these values.
    • Add support for additional custom manifest input via extraObjects value.
  • Add unittests for chart templates.
  • Various Chart.yaml metadata changes.
    • Fix logo URL.
    • Add suggestions for compatible product charts.
    • Add annotation to include source image used in pod.
  • Update README.md and other documentation to reflect changes.

@ianpittwood ianpittwood requested a review from bschwedler May 15, 2025 19:25
@ianpittwood ianpittwood marked this pull request as draft May 15, 2025 20:30
Copy link
Contributor

@t-margheim t-margheim left a comment

Choose a reason for hiding this comment

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

LGTM! I did leave a couple notes about maybe dropping RetentionPeriod from the docs to match a change on our side.

Copy link
Contributor

@markrtucker markrtucker left a comment

Choose a reason for hiding this comment

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

Looks good. I agree with Tim's comments - especially the ones around retention period

Copy link
Contributor

@t-margheim t-margheim left a comment

Choose a reason for hiding this comment

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

LGTM!

@bschwedler bschwedler force-pushed the chronicle/chart-cleanup branch from a581ace to d7f9962 Compare May 23, 2025 17:54
@bdeitte
Copy link
Member

bdeitte commented May 26, 2025

I tried to run this locally with minikube, and I ran into the issue I'm assuming with the docker containers not being updated that I didn't try to solve. I also tried to do a review here, but I'm not a helm expert. Things looked good from what I could see in my limited review.

@ianpittwood ianpittwood requested review from bdeitte and bschwedler May 27, 2025 16:33
@ianpittwood ianpittwood force-pushed the chronicle/chart-cleanup branch from f73c1a0 to fb95c66 Compare May 27, 2025 20:14
@bdeitte
Copy link
Member

bdeitte commented May 28, 2025

For anybody else curious (other than Ian and Ben who I am sure know this), the one test is failing for a good reason. It's waiting for the latest docker images that don't run as root:

Error: container has runAsNonRoot and image will run as root (pod: "posit-chronicle-pedq9r2wzi-0_posit-test(59eec832-723d-47a6-afdf-b414012687d7)", container: posit-chronicle)

Copy link
Member

@bdeitte bdeitte left a comment

Choose a reason for hiding this comment

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

Looks great to me

@ianpittwood ianpittwood force-pushed the chronicle/chart-cleanup branch from 0545eec to b58d31d Compare June 4, 2025 16:29
@bschwedler bschwedler dismissed their stale review June 5, 2025 14:19

We have changed our approach for storage since my review.

@ianpittwood ianpittwood marked this pull request as ready for review June 9, 2025 16:30
@bschwedler bschwedler merged commit ddc7657 into main Jun 9, 2025
7 checks passed
@bschwedler bschwedler deleted the chronicle/chart-cleanup branch June 9, 2025 17:16
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.

6 participants