target manifest-templates

[rajatchopra]

pkg/asset: new target manifest-templates

1. Move files from manifests/content to templates directory
2. Create new asset called templates that the target manifest-templates can directly call
3. All template files are separate assets by themselves, and 'templates' asset depends on all leaf template assets
4. Manifest/tectonic assets now use templates as parent assets that they depend upon
   Other templates (e.g. ignition/machines) are not moved into assets in this commit.

Ref: CORS-849

[abhinavdahiya]

can we make the template assets store a template type field than the consumer taking the bytes and converting to template?

[rajatchopra]

can we make the template assets store a template type field than the consumer taking the bytes and converting to template?

You mean a wrapper function to Files?
i.e.

func (t *abcdTemplateStruct) Template() (*template.Template, error) {
     return template.Must(template.New().Parse(t.Files()[0].Data))
}

Do you also mean to persist the template object itself in the struct (unexported obviously)?

[abhinavdahiya]

can we make the template assets store a template type field than the consumer taking the bytes and converting to template?

You mean a wrapper function to Files?
i.e.

func (t *abcdTemplateStruct) Template() (*template.Template, error) {
     return template.Must(template.New().Parse(t.Files()[0].Data))
}

Do you also mean to persist the template object itself in the struct (unexported obviously)?

No.

type tmplAsset struct {
    Raw []bytes
    Tmpl *template.Template
}

[rajatchopra]

type tmplAsset struct {
    Raw []bytes
    Tmpl *template.Template
}

Which just means the Files() function returns the File List extemporaneously using Raw instead. No problem.
The above example will serialize both Raw and the template.Template object . You do mean that we keep one of them unexported?
Some of the assets are not templates of course.

[crawford]

Can you rebase on top of #510? That is the direction we are going to go for assets.

[rajatchopra]

@crawford @wking Can we get a lgtm on this? The tests passed as well.

[abhinavdahiya]

@rajatchopra is this PR not bringing in changes from #510 ?

[rajatchopra]

@rajatchopra is this PR not bringing in changes from #510 ?

That one looks at ignition config templates. This one at operator manifest templates. I took the idea and structure from 510. There isn't any code overlap really.

[wking]

nits: "uri" -> "URI", "its" -> "it's".

[rajatchopra]

Done.

[wking]

All of your getFileContents calls currently include an explicit path.Join(dataDir, ...). Maybe just drop dataDir and push that Join down into getFileContents?

[rajatchopra]

Done.

[abhinavdahiya]

@rajatchopra is this PR not bringing in changes from #510 ?

That one looks at ignition config templates. This one at operator manifest templates. I took the idea and structure from 510. There isn't any code overlap really.

will /lgtm #510 then.

[rajatchopra]

Your commit message has:

data/data/templates: move all yaml content to its own files

which seems stale:

$ git show --oneline --stat 7c2d2ecde1 | grep data/data/template
...no hits...

Did you mean data/data/manifests?

Yes. That was a mistake. Thanks. Fixed. It is data/data/manifests now.

[rajatchopra]

@abhinavdahiya @wking Ready for review again.

[wking]

@rajatchopra: some YAML lint issues now that the linter has teeth again ;).

[rajatchopra]

@rajatchopra: some YAML lint issues now that the linter has teeth again ;).

Yeah.. but also because most of the files are templates :(. I should rename them to .yaml.template, fun again.

[wking]

And you might need to follow my .template suffix pattern (e.g. data/data/bootstrap/files/opt/tectonic/manifest-overrides/kube-proxy-kubeconfig.yaml.template) to avoid YAML complaining about:

./data/data/manifests/bootkube/kube-system-secret-etcd-client.yaml
  8:14      error    too many spaces inside braces  (braces)
  8:30      error    too many spaces inside braces  (braces)
  9:14      error    too many spaces inside braces  (braces)
  9:29      error    too many spaces inside braces  (braces)

for things like:

apiVersion: v1
kind: Secret
metadata:
  name: etcd-client
  namespace: kube-system
type: SecretTypeTLS
data:
  tls.crt: {{ .EtcdClientCert }}
  tls.key: {{ .EtcdClientKey }}

The YAML linter doesn't like the {{, because it's not YAML until the template gets rendered.

[wking]

Still some YAML-lint issues after the .template rename.

[wking]

It would be nice if these could live under data/data/bootstrap/files/opt/tectonic/ instead of in data/data/manifests/; there doesn't feel like there's a lot of space between, say, data/data/bootstrap/files/opt/tectonic/manifest-overrides/kube-proxy-kubeconfig.yaml.template and data/data/manifests/bootkube/kube-system-configmap-root-ca.yaml.template. But I'm fine landing this as it stands and revisiting that later.

[rajatchopra]

Still some YAML-lint issues after the .template rename.

My local yamllinter is less strict. Either way, final push, hopefully.

It would be nice if these could live under data/data/bootstrap/files/opt/tectonic/ instead of in data/data/manifests/; there doesn't feel like there's a lot of space between, say, data/data/bootstrap/files/opt/tectonic/manifest-overrides/kube-proxy-kubeconfig.yaml.template and data/data/manifests/bootkube/kube-system-configmap-root-ca.yaml.template. But I'm fine landing this as it stands and revisiting that later.

Yeah, I can catch that quickly after this one. I am in a hurry to get this one in so that I do not have to do any of the ugly rebases if anything conflicting gets in first. Just because this is an xxl PR.

[abhinavdahiya]

/approve

[wking]

/lgtm

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abhinavdahiya, rajatchopra, wking

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [abhinavdahiya,rajatchopra,wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment