Try to bring kaniko implementation up-to-date with the latest lifecycle #19
Conversation
| logrus.Debug("Check if DOCKER_FILE_NAME env is defined...") | ||
| b.DockerFileName = util.GetValFromEnVar(DOCKER_FILE_NAME_ENV_NAME) | ||
| if b.DockerFileName == "" { | ||
| b.DockerFileName = defaultDockerFileName | ||
| } | ||
| logrus.Debugf("DockerfileName is: %s", b.DockerFileName) |
There was a problem hiding this comment.
Removed as this information will be supplied from /layers/config/metadata.toml
There was a problem hiding this comment.
OK. So we have to review the README.md file to be sure that we do not support/use anymore a dockerfile but instead a TOML file containing the Dockerfile(s)
There was a problem hiding this comment.
Makes sense. As the "spec requirements" for the extender are still very much in flux I wonder if it makes sense to have another place where we could iterate on it more easily - I started a document here to try to start to formalize this. When it's a little more solidified we could put it in the repo, what do you think?
There was a problem hiding this comment.
(note that the code will still need to be updated further ...forthcoming... to reflect what's in the document, but I think this is closer to where we want to land WRT variable names, etc.)
| logrus.Debug("Checking if CNB_* env var have been declared ...") | ||
| b.CnbEnvVars = util.GetCNBEnvVar() | ||
| logrus.Debugf("CNB ENV var is: %s", b.CnbEnvVars) | ||
|
|
||
| // Convert the CNB ENV vars to Kaniko BuildArgs | ||
| for k, v := range b.CnbEnvVars { | ||
| logrus.Debugf("CNB env key: %s, value: %s", k, v) | ||
| arg := k + "=" + v | ||
| b.BuildArgs = append(b.BuildArgs, arg) | ||
| } | ||
|
|
||
| // setup the path to access the Dockerfile within the workspace dir | ||
| dockerFilePath := b.WorkspaceDir + "/" + b.DockerFileName |
There was a problem hiding this comment.
README should also remove this section explaining how to pass parameters using TOML Dockerfiles and not ENV Vars ;-)
…s of the lifecycle This change also loops over all Dockerfiles found in /layers/config/metadata.toml
natalieparellano
force-pushed
the
extender-poc
branch
from
d867144
to
51733f5
Compare
| ) | ||
|
|
||
| const ( | ||
| homeDir = "/" | ||
| kanikoDir = "/kaniko" | ||
| cacheDir = "/cache" | ||
| cacheDir = "/layers/kaniko" |
There was a problem hiding this comment.
Is this path customizable using Lifecycle ? If yes, then we should define an ENV var and externalize it
| logrus.Debug("Check if DOCKER_FILE_NAME env is defined...") | ||
| b.DockerFileName = util.GetValFromEnVar(DOCKER_FILE_NAME_ENV_NAME) | ||
| if b.DockerFileName == "" { | ||
| b.DockerFileName = defaultDockerFileName | ||
| } | ||
| logrus.Debugf("DockerfileName is: %s", b.DockerFileName) |
There was a problem hiding this comment.
OK. So we have to review the README.md file to be sure that we do not support/use anymore a dockerfile but instead a TOML file containing the Dockerfile(s)
| logrus.Debug("Checking if CNB_* env var have been declared ...") | ||
| b.CnbEnvVars = util.GetCNBEnvVar() | ||
| logrus.Debugf("CNB ENV var is: %s", b.CnbEnvVars) | ||
|
|
||
| // Convert the CNB ENV vars to Kaniko BuildArgs | ||
| for k, v := range b.CnbEnvVars { | ||
| logrus.Debugf("CNB env key: %s, value: %s", k, v) | ||
| arg := k + "=" + v | ||
| b.BuildArgs = append(b.BuildArgs, arg) | ||
| } | ||
|
|
||
| // setup the path to access the Dockerfile within the workspace dir | ||
| dockerFilePath := b.WorkspaceDir + "/" + b.DockerFileName |
There was a problem hiding this comment.
README should also remove this section explaining how to pass parameters using TOML Dockerfiles and not ENV Vars ;-)
| err = reapChildProcesses() | ||
|
|
||
| logrus.Info("Finding base image to provide ...") | ||
| baseimage := os.Getenv("CNB_BASE_IMAGE") |
There was a problem hiding this comment.
Is it documented how to pass such ENV var part of the README ? Have you reviewed the Job.yaml definition to align it with your PR - https://github.com/redhat-buildpacks/poc/blob/main/helm/templates/job.yaml#L20-L49 ?
| // Define opts | ||
| opts := b.Opts | ||
| opts.DockerfilePath = d.Path | ||
| opts.BuildArgs = toMultiArg(d.Args) |
There was a problem hiding this comment.
So finally we will only parse the Build atgs and not the run args here as I see that you changed the model ?
|
The POC on buildpacks/lifecycle#802 has changed enough that this PR is no longer useful. |
I can confirm that this builds, but not sure if it runs :)