Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change daemonset images name to match the built one #108

Conversation

abdallahyas
Copy link
Contributor

@abdallahyas abdallahyas commented Apr 11, 2021

This patch change the antrea.yml image to match the built image.
This is needed because the latest antrea.yml uses different image
than the image name that was used when scripting the CI.

@abdallahyas abdallahyas force-pushed the ariov-antrea-change-image-name branch 4 times, most recently from 8cd9c5f to 4f21f52 Compare April 12, 2021 08:38
@abdallahyas abdallahyas marked this pull request as draft April 12, 2021 08:39
@abdallahyas abdallahyas force-pushed the ariov-antrea-change-image-name branch 2 times, most recently from 2d5e439 to 973430e Compare April 12, 2021 10:43
@abdallahyas abdallahyas marked this pull request as ready for review April 12, 2021 12:39
@abdallahyas
Copy link
Contributor Author

This PR should not affect the network operator CI, and only affects the sriov_antrea which passed the CI, and i think it is ready for review

local file="$3"

let doc_num=0
changed="false"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found would serve as better variable for this case

if [[ "$kind" == "DaemonSet" ]];then
name=$(yq r -d "$doc_num" $file metadata.name)
if [[ "$name" == "$daemonset_name" ]];then
let container_index=0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to refactor it to be more readable

if [[ "$kind" == "DaemonSet" ]];then
            name=$(yq r -d "$doc_num" $file metadata.name)
            .............

to

if [[ "$kind" != "DaemonSet" ]];then
            continue
...

change_image_name antrea/antrea-ubuntu antrea/antrea-ubuntu:ci

change_k8s_resource "DaemonSet" "antrea-agent" spec.template.spec.containers[*].image "antrea/antrea-ubuntu:ci" "$WORKSPACE/antrea-cni/build/yamls/antrea.yml"
change_k8s_resource "DaemonSet" "antrea-agent" spec.template.spec.initContainers[*].image "antrea/antrea-ubuntu:ci" "$WORKSPACE/antrea-cni/build/yamls/antrea.yml"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using kustomize may help you simplify this, WDYT?

@abdallahyas abdallahyas marked this pull request as draft April 13, 2021 10:56
@abdallahyas abdallahyas force-pushed the ariov-antrea-change-image-name branch from 973430e to bd7bafb Compare April 14, 2021 06:44
@abdallahyas abdallahyas marked this pull request as ready for review April 14, 2021 06:48
@abdallahyas
Copy link
Contributor Author

@Mmduh-483 changed the patch so that it will be a simpler and more straight forward fix. please have a look

@abdallahyas abdallahyas marked this pull request as draft April 14, 2021 07:47
This patch change the antrea.yml image to match the built image.
This is needed because the latest antrea.yml uses different image
than the image name that was used when scripting the CI.
@abdallahyas abdallahyas force-pushed the ariov-antrea-change-image-name branch from bd7bafb to 1b4488a Compare April 14, 2021 07:51
@abdallahyas abdallahyas marked this pull request as ready for review April 14, 2021 09:12
cp $WORKSPACE/antrea-cni/build/yamls/antrea.yml $ARTIFACTS

sed -i "s;\(^ *image\:\).*;\1 antrea/antrea-ubuntu;" $ARTIFACTS/antrea.yml

if [[ -z "$(grep hw-offload $WORKSPACE/antrea-cni/build/yamls/antrea.yml)" ]];then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think it is more clear to do grep in $ARTIFACTS folder.
Maybe it will be better to use --hw-offload as pattern for grep?
if condition can be simplified to remove subshell usage

if ! grep -q  --hw-offload $ARTIFACTS/antrea-cni/build/yamls/antrea.yml; then
...
fi

@abdallahyas abdallahyas marked this pull request as draft April 15, 2021 07:59
@abdallahyas
Copy link
Contributor Author

@Mmduh-483 @ykulazhenkov can you have a look at antrea-io/antrea#2077
I am thinking of deprecating the project in this repo, since it is mainly hosted at the antrea repo itself and just keep it here as a reference. WDYT?

@Mmduh-483
Copy link
Contributor

@Mmduh-483 @ykulazhenkov can you have a look at vmware-tanzu/antrea#2077

Won't this affect PR and daily CIs?

@abdallahyas
Copy link
Contributor Author

abdallahyas commented Apr 15, 2021

@Mmduh-483 @ykulazhenkov can you have a look at vmware-tanzu/antrea#2077

Won't this affect PR and daily CIs?

PRs in the antrea already use that, dailies should be modified to use that as well, regarding the CI checks, they can be dropped for this project

@abdallahyas
Copy link
Contributor Author

closed since it was fixed in the antrea repo directly

@abdallahyas abdallahyas closed this May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants