Use existing helm template logic to populate manifests instead of dry-run #1718
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
1 similar comment
Does the PR have any schema changes?Looking good! No breaking changes found. |
viveklak
changed the title
Does the PR have any schema changes?Looking good! No breaking changes found. |
There was a problem hiding this comment.
This makes sense to me, would love @lblackstone to take a look too
| repo: "https://prometheus-community.github.io/helm-charts", | ||
| }, | ||
| values: { | ||
| commonLabels: { |
There was a problem hiding this comment.
Nit: add a comment somewhere about the purpose of this test (what step2 is verifying)
viveklak
force-pushed
the
vl/HelmReleaseCRD
branch
from
d628aa4
to
4a2a3ce
Compare
| const prometheusRelease = new k8s.helm.v3.Release("prometheus", { | ||
| name: "kube-prometheus-stack", | ||
| chart: "kube-prometheus-stack", | ||
| repositoryOpts: { |
There was a problem hiding this comment.
We should probably pin to a specific version just in case upstream breaks something in the future.
Does the PR have any schema changes?Looking good! No breaking changes found. |
There was a problem hiding this comment.
This worked when I tested it locally, but I had a couple questions:
- Are we planning to render the manifests in a separate PR? We should have all the info we need to do that now similarly to our Chart support.
- I got a number of warnings during the update. Any idea where those are coming from?
E0917 10:12:09.637652 63178 memcache.go:196] couldn't get resource list for monitoring.coreos.com/v1alpha1: the server could not find the requested resource
E0917 10:12:09.748362 63178 memcache.go:196] couldn't get resource list for monitoring.coreos.com/v1: the server could not find the requested resource
E0917 10:12:09.777696 63178 memcache.go:196] couldn't get resource list for monitoring.coreos.com/v1: the server could not find the requested resource
E0917 10:12:09.839002 63178 memcache.go:196] couldn't get resource list for monitoring.coreos.com/v1: the server could not find the requested resource
This is tracked in #1701. We have had that support with dry-run as well but maps were not rendering. I can try rendering as formatted yaml instead but that would be in the context of a separate PR. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Thanks for pointing that out. I saw them as well. Looks like these were spurious log messages from the stock discovery cache client around CRD installs (perhaps defaulting to stdout). Switched to version in clients and the log messages are gone. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
Does the PR have any schema changes?Looking good! No breaking changes found. |
|
There seems to be something going on with the CI run - Levi and I have independently verified the new test locally and I have run it on an identical GKE cluster to the CI environment. All of them work but it seems the released provider is being somehow pulled into the test run on GHA. I am going to force merge this and watch the master build to see what is up. |
Fixes #1712
This avoids issues for charts with CRDs which don't play well with dry-run.