kubebuilder
kubebuilder copied to clipboard
:sparkles: (kustomize/v1 and kustomize/v2-alpha): add labels on the manifests following the k8s recommendations
Description of Change
update or add labels in the kustomize plugin(s) scaffolded files to match the k8s recommended labels
Motivation for Change
resolves #2773
Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all
/hold
/cc @camilamacedo86
@camilamacedo86 I will make sure to squash the commits before merge, but wanted to keep as individual commits until all the labels we want are in place and there are only minor adjustments to be made.
/retest
@everettraven for this PR because it is so large, it would be helpful to give a synopsis TL;DR of what the change is doing.
Hi @jmrodri,
@everettraven for this PR because it is so large, it would be helpful to give a synopsis TL;DR of what the change is doing.
It is big because we are adding the labels to follow the k8s recommendations in all templates and it will update ALL samples. Following is an example with comments from the definition in the k8s docs:
metadata:
labels:
app.kuberentes.io/name: issuer // The name of the application ( in this case the Kind )
app.kubernetes.io/instance: selfsigned-issuer // A unique name identifying the instance of an application (CR)
app.kubernetes.io/component: certificate // The component within the architecture
app.kubernetes.io/created-by: {{ .ProjectName }} // The controller/user who created this resource (in our case it is not created by a controller and it is manage by the kind )
app.kubernetes.io/part-of: {{ .ProjectName }} // The name of a higher level application this one is part of
app.kubernetes.io/managed-by: cert-manager.io // The tool being used to manage the operation of an application - @everettraven this one is wrong. The tool in the example is helm. Here we manage the manifest with kustomize.
But, note that GitHub allow you to check the directories which have manifest changed and you can ignore all under the testdata in this case ( because these files are the samples changed ):
How to review? a) Check the doc: https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/ b) Check the manifests changed and the labels ( ignoring the testdata ) and see if you agree with the usage/values adopted by the labels.
c/c @everettraven
Hi @everettraven,
The app.kubernetes.io/managed-by: cert-manager.io is wrong, see the docs that is the Tool that manages the manifest in our case the manifests can only be managed with kustomize or with the project itself via the controllers. the cert-manager.io is NOT a tool it is a service.
So, we should here add kustomize for all manifest under config/ because that is the tool ( such as helm ) that is used to build the files.
/hold
Hi @everettraven
For we get this one merged we need to squash the commits? Could you please squash them?
/hold
@camilamacedo86 commits squashed!
@camilamacedo86 the code coverage failure seems to be unrelated to this PR and may just need a re-run. Would you be able to re-run it? I don't seem to have permissions to do so.
@camilamacedo86 I rebased this PR and I think it should be good to go now. Would you mind giving it another look so we can get it merged? Thanks!
/unhold
/cc @varshaprasad96 @jmrodri
@camilamacedo86 I rebased this PR and I think it should be good to go now. Would you mind giving it another look so we can get it merged? Thanks!
@camilamacedo86 - I just wanted to follow up on this PR. Thanks!
The one thing that would have made this easier to review is if all of the testdata files were a single commit and the main changes were another. Then I could take a quick look at testdata but focus on the core changes. Just an FYI for next time.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: AlmogBaku, camilamacedo86, everettraven
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [camilamacedo86]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment