🐛 (kustomize/v2-alpha): use marker to track samples and do not create tracks for core types #3102
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
the
size/L
camilamacedo86
changed the title
|
/lgtm |
k8s-ci-robot
added
the
lgtm
| @@ -95,6 +82,13 @@ func (s *apiScaffolder) Scaffold() error { | |||
| ); err != nil { | |||
| return fmt.Errorf("error scaffolding kustomize API manifests: %v", err) | |||
| } | |||
|
|
|||
| // If the gvk is non-empty | |||
| if s.resource.Group != "" || s.resource.Version != "" || s.resource.Kind != "" { | |||
There was a problem hiding this comment.
nits: Just wondering in which case this gvk might be empty so that this sample kustomization should be prevented?
There was a problem hiding this comment.
It is only to prevent somewhere the validation was not made before.
I believe that when we call create API and say that we want only the controller if we do not add the check here it will create a duplication.
| if err := scaffold.Execute(&samples.Kustomization{}); err != nil { | ||
| return fmt.Errorf("error scaffolding manifests: %v", err) | ||
| } | ||
| } |
There was a problem hiding this comment.
nits: we may remove the method generateManifestsPath as it is unused.
| func (f *Kustomization) SetTemplateDefaults() error { | ||
| if f.Path == "" { | ||
| f.Path = filepath.Join("config", "samples", "kustomization.yaml") | ||
| } | ||
| f.Path = f.Resource.Replacer().Replace(f.Path) |
There was a problem hiding this comment.
I'm wondering why we'd like to update f.Path here? Will it be values other than config/samples/kustomization.yaml by calling replacer?
There was a problem hiding this comment.
Good catcher, we do not need this here.
| // GetCodeFragments implements file.Inserter | ||
| func (f *Kustomization) GetCodeFragments() machinery.CodeFragmentsMap { | ||
| return machinery.CodeFragmentsMap{ | ||
| machinery.NewMarkerFor(f.Path, samplesMarker): []string{fmt.Sprintf(samplesCodeFragment, f.makeCRFileName())}, |
There was a problem hiding this comment.
nits: will it be good/necessary if we declare a new method like:
func (f *Kustomization) sampleMarker() machinery.Marker {
return machinery.NewMarkerFor(f.Path, samplesMarker)
}So that we may keep consistency to use sample marker in various places in the file?
There was a problem hiding this comment.
I this case I think we only use here. right? But we could improve.
…se marker instead of overwritting
camilamacedo86
force-pushed
the
fix-kust
branch
from
e88e238
to
e615d6a
Compare
k8s-ci-robot
removed
the
lgtm
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, everettraven 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 |
Description
Instead of overwriting the file, we can use a marker so we will just append new values instead.
Motivations