Creating a Helm chart and repository for GMSA. #55
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/XL
|
/assign @jsturtevant |
There was a problem hiding this comment.
Thanks! Like seeing the integration with Cert manager.
Can we add a few things here:
- a test that validates the chart deploys and runs against integration with cert manager. something like: with a flag for cert manager.
windows-gmsa/.github/workflows/build.yaml
Line 24 in 9fa8e91
- ability to generate the final yaml from the make file, this will help make sure the chart and yaml file don't get out sync, I've done this type of thing before https://github.com/Azure/azure-k8s-metrics-adapter/blob/master/hack/gen-deploy.sh and https://github.com/Azure/azure-k8s-metrics-adapter/blob/master/deploy/README.md as example.
phillipsj
force-pushed
the
feature/helm-chart
branch
from
d3fca72 to
48617ad
Compare
|
Linking this issue. |
k8s-ci-robot
added
the
sig/windows
phillipsj
force-pushed
the
feature/helm-chart
branch
from
5d7fb63 to
fd00e98
Compare
|
/cc @marciogmorales for the cert-manage changes |
|
@jsturtevant: GitHub didn't allow me to request PR reviews from the following users: changes, marciogmorales, for, the. Note that only kubernetes-sigs members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
phillipsj
force-pushed
the
feature/helm-chart
branch
from
2783858 to
53734a4
Compare
k8s-ci-robot
added
the
needs-rebase
Created a Helm chart for GMSA that supports installing the CRD, CertManager, and the Admission web hook. This also makes this repository a Helm chart repo. Signed-off-by: Jamie Phillips <[email protected]>
Just stubbing out all of the items needed. Signed-off-by: Jamie Phillips <[email protected]>
Signed-off-by: Jamie Phillips <[email protected]>
Signed-off-by: Jamie Phillips <[email protected]>
Signed-off-by: Jamie Phillips <[email protected]>
phillipsj
force-pushed
the
feature/helm-chart
branch
from
708c096 to
c56fc34
Compare
k8s-ci-robot
removed
the
needs-rebase
Signed-off-by: Jamie Phillips <[email protected]>
Signed-off-by: Jamie Phillips <[email protected]>
Signed-off-by: Jamie Phillips <[email protected]>
|
@jsturtevant made some progress. The chart integration test is failing for some odd reason, but it works on the others. I will be digging into that a little later. |
.github/workflows/build.yaml
Outdated
| - name: Set up Go | ||
| uses: actions/setup-go@v2 | ||
| with: | ||
| go-version: 1.16 |
There was a problem hiding this comment.
can you try bumping this to 1.17.
I've seen the same errors you are seeing in CI job when trying to build w/ go 1.16 when deps moved to 1.17.
Error: /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/internal/golang/encoding/json/encode.go:1249:12: sf.IsExported undefined (type reflect.StructField has no field or method IsExported)
Error: /home/runner/go/pkg/mod/sigs.k8s.io/[email protected]/internal/golang/encoding/json/encode.go:1255:18: sf.IsExported undefined (type reflect.StructField has no field or method IsExported)
Plus all the other jobs in this file are using 1.17 :)
There was a problem hiding this comment.
@marosset just needed that extra set of eyes. Thanks.
|
Looks like the go version bump fixed the CI issue. |
|
/label tide/merge-method-squash |
k8s-ci-robot
added
the
tide/merge-method-squash
| @@ -16,6 +16,13 @@ If your repo has certain guidelines for contribution, put them here ahead of the | |||
| - [Kubernetes Contributor Guide](http://git.k8s.io/community/contributors/guide) - Main contributor documentation, or you can just jump directly to the [contributing section](http://git.k8s.io/community/contributors/guide#contributing) | |||
| - [Contributor Cheat Sheet](https://git.k8s.io/community/contributors/guide/contributor-cheatsheet.md) - Common resources for existing developers | |||
|
|
|||
| ## Generating Helm Charts and Index | |||
|
|
|||
| When a chart needs to be updated, create the new version and the chart information. Run helm pack, then generate a new Helm chart index.yaml with the following command. | |||
There was a problem hiding this comment.
We could/should add a CI check to see if helm charts need to be rebuilt.
This can be done as a seperate change.
|
Looks like most of the feedback has been addressed and CI job is passing. |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: marosset, phillipsj The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Created a Helm chart for GMSA that supports installing the CRD, CertManager, and the Admission web hook. This also makes this repository a Helm chart repo.
Signed-off-by: Jamie Phillips [email protected]