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

Update 6.5-gateway-certgen-job.yaml #5350

Merged
merged 3 commits into from
Sep 20, 2021
Merged

Conversation

jwilner
Copy link
Contributor

@jwilner jwilner commented Sep 18, 2021

Description

Remove explicit restartPolicy definition, since it's already handled by gloo.podSpecStandardFields, and the duplicate key results in YAML that is invalid for stricter tools (like kustomize).

Context

Unable to use kustomize helm chart inflator with gloo-ee charts because of this duplicate field

STR:

$ cd $(mktemp -d)
$ cat <<EOF > kustomization.yaml
helmCharts:
  - name: gloo-ee
    version: 1.8.12
    releaseName: gloo
    repo: https://storage.googleapis.com/gloo-ee-helm
EOF
$ kubectl kustomize --enable-helm
Error: map[string]interface {}(nil): yaml: unmarshal errors:
  line 38: mapping key "restartPolicy" already defined at line 21
$ kubectl version
Client Version: version.Info{Major:"1", Minor:"22", GitVersion:"v1.22.1", GitCommit:"632ed300f2c34f6d6d15ca4cef3d3c7073412212", GitTreeState:"clean", BuildDate:"2021-08-19T15:38:26Z", GoVersion:"go1.16.6", Compiler:"gc", Platform:"darwin/amd64"}
Server Version: version.Info{Major:"1", Minor:"21", GitVersion:"v1.21.1", GitCommit:"5e58841cce77d4bc13713ad2b91fa0d961e69192", GitTreeState:"clean", BuildDate:"2021-05-21T23:01:33Z", GoVersion:"go1.16.4", Compiler:"gc", Platform:"linux/amd64"}

Checklist:

  • I included a concise, user-facing changelog (for details, see https://github.com/solo-io/go-utils/tree/master/changelogutils) which references the issue that is resolved.
  • If I updated APIs (our protos) or helm values, I ran make -B install-go-tools generated-code to ensure there will be no code diff
  • I followed guidelines laid out in the Gloo Edge contribution guide
  • I opened a draft PR or added the work in progress label if my PR is not ready for review
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

BOT NOTES:
resolves #5350

Remove explicit restartPolicy definition, since it's already handled by `gloo.podSpecStandardFields`, and the duplicate key results in YAML that is invalid for stricter tools (like kustomize).
@solo-build-bot
Copy link

Waiting for approval from someone in the solo-io org to start testing.

@jwilner
Copy link
Contributor Author

jwilner commented Sep 18, 2021

Also, it'd be great if y'all could set up CI that verifies that the chart would be suitable for kustomize (i.e. pretty much just running the above ^). I'm happy to help set it up if you want to point me at the right place.

Longer term, would also love for there to be a flat manifest for installation so helm didn't have to be involved at all :)

@sam-heilbron
Copy link
Contributor

/test

@sam-heilbron
Copy link
Contributor

@jwilner thanks for the contribution!

Can you add a changelog file, following the steps outlined here: https://github.com/solo-io/go-utils/tree/master/changelogutils#changelog-files? Once that is added, this looks good to me.

Also, it'd be great if y'all could set up CI that verifies that the chart would be suitable for kustomize (i.e. pretty much just running the above ^). I'm happy to help set it up if you want to point me at the right place.

Longer term, would also love for there to be a flat manifest for installation so helm didn't have to be involved at all :)

Our helm tests live in 2 places:

A contribution to support kustomize validation would be great! Let me know if you need any other pointers for where to get started

@solo-changelog-bot
Copy link

Issues linked to changelog:
#5350

@jwilner
Copy link
Contributor Author

jwilner commented Sep 20, 2021

Thanks @sam-heilbron. I'll see if I can get to the kustomize tests soon.

@sam-heilbron
Copy link
Contributor

Thank you! I see this bug was introduced in gloo 1.8. Could you backport your fix to the 1.8.x branch as well @jwilner ?

@jwilner jwilner mentioned this pull request Sep 20, 2021
@jwilner
Copy link
Contributor Author

jwilner commented Sep 20, 2021

@sam-heilbron -- done. See #5353.

@sam-heilbron
Copy link
Contributor

/test

@soloio-bulldozer soloio-bulldozer bot merged commit 34c21d7 into solo-io:master Sep 20, 2021
soloio-bulldozer bot pushed a commit that referenced this pull request Sep 20, 2021
* Update 6.5-gateway-certgen-job.yaml
* Create duplicate-key-helm.yaml
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.

None yet

2 participants