Remove unified image #6182
Remove unified image #6182
Conversation
From now on, we will only publish the new "split" images (e.g., antrea/antrea-agent-ubuntu and antrea/antrea-controller-ubuntu). For antrea-io#4832 Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
| @@ -457,22 +457,36 @@ function deliver_antrea { | |||
| fi | |||
|
|
|||
| echo "====== Pulling old Antrea images ======" | |||
There was a problem hiding this comment.
@XinShuYang @tnqn is this code used?
The "upgrade" test in this file is surprising to me, since we use an older Antrea image for some Nodes, but AFAIK we use a single manifest. So I guess we are testing "some new agents + some old agents + one new controller"?
There was a problem hiding this comment.
AFAIK OLD_ANTREA_VERSION is only used in matrix test, but as we discussed in #6161 this test has not been used for a long time.
There was a problem hiding this comment.
I was unaware of the code and the test. Version skew scenario can be covered by GIthub job test-compatible-N-1 and test-compatible-N-2. So it should be fine to remove this if it hasn't been used.
There was a problem hiding this comment.
Thanks. I will keep the changes I made even though they will be untested, and I assume this script will be updated correctly as part of #6161
| @@ -34,10 +34,8 @@ jobs: | |||
| run: | | |||
| ./hack/build-antrea-linux-all.sh --pull | |||
| echo "$DOCKER_PASSWORD" | docker login -u "$DOCKER_USERNAME" --password-stdin | |||
| docker tag antrea/antrea-ubuntu:"${VERSION}" antrea/antrea-ubuntu-amd64:"${VERSION}" | |||
There was a problem hiding this comment.
Should we also removed the line here?
https://github.com/antrea-io/antrea/blob/main/ci/jenkins/mellanox/scripts/start_ci.sh#L114
There was a problem hiding this comment.
I will remove the line, but I think we should consider removing the script... as it is no longer maintained AFAIK
There was a problem hiding this comment.
The script is definitely broken too, it still uses the Harbor name for the Antrea docker images
| @echo "===> Building Antrea bins and antrea/antrea-ubuntu-coverage Docker image <===" | ||
| docker build -t antrea/antrea-ubuntu-coverage:$(DOCKER_IMG_VERSION) -f build/images/Dockerfile.build.coverage $(DOCKER_BUILD_ARGS) . | ||
| docker tag antrea/antrea-ubuntu-coverage:$(DOCKER_IMG_VERSION) antrea/antrea-ubuntu-coverage | ||
| build-ubuntu-coverage: build-controller-ubuntu-coverage build-agent-ubuntu-coverage |
There was a problem hiding this comment.
I think that we can remove this now, because it is called only during test from build-antrea-linux-all.sh, and there we are already calling the build-agent-ubuntu and build-controller-ubuntu
There was a problem hiding this comment.
I have a comment to the Makefile explaining that I am keeping the targets for backwards compatibility. It's just an alias and someone / a script outside of the repo may be using the target directly?
| # This target is for development only. It assumes that "make bin" has been run previously and will | ||
| # copy the local binaries to the Docker image, instead of building the binaries inside the image as | ||
| # part of the Docker build. | ||
|
|
||
| .PHONY: ubuntu | ||
| ubuntu: |
There was a problem hiding this comment.
A nit, Can we remove ubuntu, and we can modify the CONTRIBUTING.md file to make use of make build-agent-ubuntu and make build-controller-ubuntu instead of make ubuntu.
There was a problem hiding this comment.
These targets do different things. I know that some folks prefer to build the binaries externally and copy them inside the Docker container (building this way is less portable but may be faster).
| @@ -0,0 +1,23 @@ | |||
| # Copyright 2024 Antrea Authors | |||
There was a problem hiding this comment.
Just curious to know why is this file added, when we already have dockerfile.build.controller.ubuntu.
There was a problem hiding this comment.
See comment above
Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
|
/test-all |
antoninbas
added
the
action/release-note
From now on, we will only publish the new "split" images (e.g., antrea/antrea-agent-ubuntu and antrea/antrea-controller-ubuntu).
For #4832