Add CPE label and RHEL version placeholder in Dockerfile generator#791
Conversation
Should we rather include a specific template files into embed, I still see a lot of error in the generator tests. |
|
This place needs some attention: https://github.com/openshift-knative/hack/blob/main/pkg/dockerfilegen/generator.go#L281-L294 |
| import "embed" | ||
|
|
||
| //go:embed dockerfile-templates/Default.dockerfile.tmpl | ||
| //go:embed dockerfile-templates/Default.dockerfile.tmpl dockerfile-templates/rhel-9/*.tmpl |
There was a problem hiding this comment.
| //go:embed dockerfile-templates/Default.dockerfile.tmpl dockerfile-templates/rhel-9/*.tmpl | |
| //go:embed dockerfile-templates/Default.dockerfile.tmpl dockerfile-templates/rhel-9/* |
It should be dir input, otherwise pattern match with suffix doesn't work.
…p for must gather
| if rhelVersion == RHEL9 { | ||
| templateFile = "dockerfile-templates/rhel-9/Default.dockerfile.tmpl" | ||
| } else { | ||
| templateFile = "dockerfile-templates/Default.dockerfile.tmpl" | ||
| } |
There was a problem hiding this comment.
It's enough to fix embed directive to either wildcard dir. We don't need to have full path everywhere.
| templateFiles := "dockerfile-templates/*.tmpl" | ||
| if rhelVersion == "rhel-9" { | ||
| templateFiles = "dockerfile-templates/rhel-9/*.tmpl" | ||
| } | ||
| t, err := template.ParseFS(dockerfileTemplate, templateFiles) |
There was a problem hiding this comment.
With correct embed this will work as expected.
| // Pick proper template FS file and RPM lock file | ||
| templateFile := "dockerfile-templates/rhel-9/*.tmpl" // RHEL9 default | ||
| if rhelVersion == RHEL9 { | ||
| rpmsLockTemplate = &RPMsLockTemplateUbi9 | ||
| } else { | ||
| templateFile = "dockerfile-templates/*.tmpl" | ||
| rpmsLockTemplate = &RPMsLockTemplateUbi8 | ||
| } | ||
|
|
||
| t, err := template.ParseFS(DockerfileMustGatherTemplate, templateFile) | ||
| if err != nil { | ||
| return fmt.Errorf("%w: Parsing failed: %w", | ||
| ErrBadTemplate, errors.WithStack(err)) | ||
| } | ||
|
|
||
| bf := new(bytes.Buffer) | ||
| if err := t.Execute(bf, d); err != nil { | ||
| return fmt.Errorf("%w: executing failed: %w", | ||
| ErrBadTemplate, errors.WithStack(err)) | ||
| } | ||
|
|
There was a problem hiding this comment.
IMO, this whole part is still extra and not needed.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dsimansk, Kaustubh-pande The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit f0ba683
into
openshift-knative:main
2 participants
cc @dsimansk