-
Notifications
You must be signed in to change notification settings - Fork 375
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
Different Images for Antrea Agent and Controller #5794
Conversation
ff03638
to
0dc0027
Compare
34ec080
to
a604e31
Compare
82b6129
to
19e3eef
Compare
/test-all |
19e3eef
to
458428d
Compare
/test-all |
458428d
to
15dcf81
Compare
/test-all |
15dcf81
to
1bc53ec
Compare
/test-all |
/test-all |
@@ -6947,7 +6947,7 @@ spec: | |||
serviceAccountName: antrea-agent | |||
initContainers: | |||
- name: install-cni | |||
image: "antrea/antrea-ubuntu:latest" | |||
image: "antrea/antrea-agent-ubuntu:latest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this PR may benefit from being broken into 2 parts. The reason is that after we merge this PR, antrea/antrea-agent-ubuntu:latest
and antrea/antrea-controller-ubuntu:latest
will not exist in the registry for a while. The workflows in this repository only push the amd64 version of the images: antrea/antrea-agent-ubuntu-amd64:latest
and antrea/antrea-controller-ubuntu-amd64:latest
. We have a separate workflow in a separate repository which takes care of the arm images AND takes care of pushing the multi-arch manifests (antrea/antrea-agent-ubuntu:latest
and antrea/antrea-controller-ubuntu:latest
).
So if we merge this PR as is, and someone tries to apply https://raw.githubusercontent.com/antrea-io/antrea/main/build/yamls/antrea.yml, I expect that it will fail.
The steps should probably be as follows:
- all the infrastructure changes required to build the split images are merged, but the Helm chart and YAML manifests are not modified yet
- I take care of updating the infrastructure which builds and pushes the ARM images and multi-arch manifests
- changes to the Helm chart and YAML manifests are merged
cc @luolanzone for visibility
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, so for this PR I can remove the YAML changes and the helm chart changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Remove them from this PR but keep them for the follow-up PR.
As soon as the 1st PR is merged, I'll update the build infra.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also the generate-manifest changes have to be reverted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but won't that affect the github and other ci jobs because with the unmodified yaml it will test the unified image and not the split images.
@antoninbas @tnqn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm concerned about users applying https://raw.githubusercontent.com/antrea-io/antrea/main/build/yamls/antrea.yml to deploy the latest Antrea, not about CI jobs which build the image themselves.
also the generate-manifest changes have to be reverted?
yes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really the first PR should only have the build infrastructure to build and push the images. It should not update any manifest or test script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1646714
to
d182a56
Compare
Modified the code to build separate images for antrea-agent and antrea-controller, because there are many resources that are not required by controller and are required by agent only, and unified image for both creates a burden when starting antrea-controller and thus it takes time to start. For this reason I have create separate images for antrea-agent and antrea-controller. Signed-off-by: Pulkit Jain <[email protected]>
Signed-off-by: Pulkit Jain <[email protected]>
d182a56
to
956a7ae
Compare
Fixes #5691.
Modified the code to build separate images for antrea-agent and antrea-controller, because there are many resources that are not required by controller and are required by agent only, and unified image for both creates a burden when starting antrea-controller and thus it takes time to start. For this reason I have create separate images for antrea-agent and antrea-controller.