NO-JIRA: fix(build): use yq to modify kustomization.yaml during deploy targets #828
Conversation
The current method of using `sed` to modify `kustomization.yaml` when running the `deploy9-` `Makefile` target runs into issues if the developer workstation is on MacOS. Given the intended behavior here is to swap out a couple values in the `yaml` file - this PR adds `yq` as a self-contained dependency to the `Makefile` to handle this `yaml` modification. The `bin/yq` target added to `Makefile` follows the existing conventions established around the `bin/kubectl` handling within the `Makefile`.
| @@ -285,17 +287,28 @@ ifeq (,$(wildcard $(KUBECTL_BIN))) | |||
| @chmod +x $(KUBECTL_BIN) | |||
| endif | |||
|
|
|||
| # Download yq binary | |||
| .PHONY: bin/yq | |||
There was a problem hiding this comment.
One point of interest here (as this merely follows the pattern established from bin/kubectl) ...
I wonder if we should actually NOT declare a .PHONY target here? as the bin/yq (and likewise bin/kubectl) are actual files on the filesystem... such that we could simply allow make to do what it does ?
pros:
- wouldn't run and download "stuff" on every invocation
cons:
- users would ned to manually intervene if we needed to upgrade the version of the binary we pull down
|
/lgtm |
Subsequent PR, maybe? and change in both places? IMO its phony because somebody did not want to deal with Makefile details and just used it as task runner |
|
/lgtm |
|
/test notebook-rocm-jupyter-pyt-ubi9-python-3-11-pr-image-mirror |
|
/lgtm |
|
@daniellutz had raised a concern that introducing In discussion during scrum - @harshad16 stated that while we are on |
|
/override ci/prow/images |
|
@andyatmiami: Overrode contexts on behalf of andyatmiami: ci/prow/images In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: andyatmiami 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
The current method of using
sedto modifykustomization.yamlwhen running thedeploy9-Makefiletarget runs into issues if the developer workstation is on MacOS.Given the intended behavior here is to swap out a couple values in the
yamlfile - this PR addsyqas a self-contained dependency to theMakefileto handle thisyamlmodification.The
bin/yqtarget added toMakefilefollows the existing conventions established around thebin/kubectlhandling within theMakefile.How Has This Been Tested?
I implemented this fix as part of my efforts working on RHOAING-12822 which is documented in part here:
I have been able to successfully deploy multiple workbench image files while running this fix. Running any
deploy9-ordeploy-c9starget will invoke the modified code.Merge criteria: