NO-ISSUE: Add steps for flannel CNI as part of OKD #4047
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@praveenkumar: This pull request explicitly references no jira issue. 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/hold Looks like only changing the release info json in case of optional assets doesn't really update the image, this image is part of kustomization ? |
openshift-ci
bot
added
the
do-not-merge/hold
|
/unhold |
openshift-ci
bot
removed
the
do-not-merge/hold
|
Put changes directly |
|
/test verify |
okd/src/use_okd_assets.sh
Outdated
| kube_proxy_okd_image_name="${kube_proxy_okd_image_with_hash%%@*}" | ||
| echo "kube-proxy image name ${kube_proxy_okd_image_name}" | ||
| kube_proxy_okd_image_hash="${kube_proxy_okd_image_with_hash##*@}" | ||
| echo "kube-proxy image hash ${kube_proxy_okd_image_hash}" |
There was a problem hiding this comment.
What are we trying to do with the name manipulation? Maybe expanding on it in the comment would be helpful.
Can we remove the echo's?
There was a problem hiding this comment.
Yes I will put comment why it is required.
okd/src/use_okd_assets.sh
Outdated
| @@ -43,6 +43,14 @@ replace_assets(){ | |||
| # update the infra pods for crio | |||
| sed -i 's,pause_image .*,pause_image = '"\"${pod_image}\""',' "packaging/crio.conf.d/10-microshift_${UNAME_TO_GOARCH_MAP[${arch}]}.conf" | |||
|
|
|||
| # kube proxy is required for flannel | |||
| kube_proxy_okd_image_with_hash=$(oc adm release info --image-for="kube-proxy" "${okd_url}:${okd_releaseTag}") | |||
| echo "kube-proxy ${kube_proxy_okd_image}" | |||
There was a problem hiding this comment.
Getting a linter warning here:
./okd/src/use_okd_assets.sh:48:22: warning: kube_proxy_okd_image is referenced but not assigned. [SC2154]
okd/src/use_okd_assets.sh
Outdated
| # kube proxy is required for flannel | ||
| kube_proxy_okd_image_with_hash=$(oc adm release info --image-for="kube-proxy" "${okd_url}:${okd_releaseTag}") | ||
| echo "kube-proxy ${kube_proxy_okd_image_with_hash}" | ||
| # The OKD image we retrieve is in the format quay.io/okd/scos-content@sha256:<hash>, | ||
| # where the image name and digest (hash) are combined in a single string. | ||
| # However, in the kustomization.${arch}.yaml file, we need the image name (newName) and | ||
| # the digest in separate fields. To achieve this, we first extract the image name and digest | ||
| # using parameter expansion, then use the sed command to insert these values into the | ||
| # appropriate places within the YAML file. | ||
| kube_proxy_okd_image_name="${kube_proxy_okd_image_with_hash%%@*}" | ||
| kube_proxy_okd_image_hash="${kube_proxy_okd_image_with_hash##*@}" | ||
| sed -i "s|newName:.*|newName: ${kube_proxy_okd_image_name}|; s|digest:.*|digest: ${kube_proxy_okd_image_hash}|" "${MICROSHIFT_ROOT}/assets/optional/kube-proxy/kustomization.${arch}.yaml" |
There was a problem hiding this comment.
Sorry for being so picky about this, but can we use yq to edit yaml files?
Something like the following (not tested):
yq eval '.images[] |= select(.name == "kube-proxy") | .newName = "${kube_proxy_okd_image_name}" | .digest = "${kube_proxy_okd_image_hash}"' -i "${MICROSHIFT_ROOT}/assets/optional/kube-proxy/kustomization.${arch}.yaml"
We can have yq installed using ./scripts/fetch_tools.sh yq
There was a problem hiding this comment.
I also prefer yq but it means adding another dep and that's the reason I went with sed.
There was a problem hiding this comment.
It should not be a problem since we have a standard way of installing yq.
We can call ./scripts/fetch_tools.sh yq in the script.
Older version of yq have issue when try to replace string it also remove empty line which is required.
This pr update readme and respective files to build flannel rpm and consume it as part of microshift multibuild container.
|
/test e2e-aws-tests-bootc-arm |
|
/lgtm |
|
/override e2e-aws-tests-bootc-arm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eslutsky, praveenkumar 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 |
openshift-ci
bot
added
the
approved
|
@eslutsky: /override requires failed status contexts, check run or a prowjob name to operate on.
Only the following failed contexts/checkruns were expected:
If you are trying to override a checkrun that has a space in it, you must put a double quote on the context. 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. |
|
/override ci/prow/e2e-aws-tests-bootc-arm |
|
@eslutsky: Overrode contexts on behalf of eslutsky: ci/prow/e2e-aws-tests-bootc-arm 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. |
|
@praveenkumar: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
This pr update readme and respective files to build flannel rpm and consume it as part of microshift multibuild container.