Merge clustermesh-apiserver and kvstoremesh into a single image #27888
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
giorio94
added
kind/cleanup
giorio94
force-pushed
the
pr/giorio94/main/clustermesh-apiserver-single-image
branch
2 times, most recently
from
85fb27e
to
06b5660
Compare
|
/test |
giorio94
force-pushed
the
pr/giorio94/main/clustermesh-apiserver-single-image
branch
from
06b5660
to
d9c7859
Compare
|
/test |
giorio94
force-pushed
the
pr/giorio94/main/clustermesh-apiserver-single-image
branch
from
d9c7859
to
578c890
Compare
|
/test |
giorio94
force-pushed
the
pr/giorio94/main/clustermesh-apiserver-single-image
branch
from
578c890
to
1578a26
Compare
|
/test |
|
Conformance Cluster Mesh hit #27672 |
giorio94
force-pushed
the
pr/giorio94/main/clustermesh-apiserver-single-image
branch
from
1578a26
to
7cb8327
Compare
|
/test |
|
Converting back to draft. I'm going to split this PR into multiple ones to reduce the size. |
This was referenced Oct 10, 2023
Refactor the clustermesh-apiserver to mimic the structure adopted by the agent, as a preparation to merge it with kvstoremesh in a single binary. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Move the current clustermesh-apiserver code to a dedicated subcommand, to prepare for the subsequent merging of kvstoremesh into the same binary. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Refactor kvstoremesh to mimic the structure adopted by the agent, as a preparation to merge it with the clustermesh-apiserver in a single binary. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
giorio94
force-pushed
the
pr/giorio94/main/clustermesh-apiserver-single-image
branch
from
7cb8327
to
3f163d0
Compare
|
/test |
|
A couple of preliminary commits have been extracted into their own PRs to reduce the size of this one, and have been merged. I've rebased this PR onto main, before marking it ready for review again. Now, it only shuffles things around, and performs the necessary adaptations. In the end, I've preserved the clustermesh-apiserver image name. The reason being that changing it would be a quite significant effort to adapt the CI, renovate configs and alike, but bringing little value, as the clustermesh-apiserver name would still used in all user-facing resources. And changing it also for the user-facing ones would be complex in terms of backward compatibility (especially concerning helm settings), and could end up bringing more confusion than clarity (as users currently recognize the clustermesh-apiserver as the control plane for clustermesh). Happy to reconsider if anyone has strong opinions against the current approach though. |
Merge the kvstoremesh logic into the clustermesh-apiserver as a separate
subcommand, so that we can get rid of one image and reduce the overall
time to bootstrap the clustermesh-apiserver deployment. The additional
boilerplate to build the kvstoremesh images is dropped, except for the
Dockerfile, which is currently preserved to prevent CI failures (and
will be removed through a subsequent commit).
Size of the images before this change:
$ make docker-clustermesh-apiserver-image
$ docker inspect -f '{{ .Size }}' quay.io/cilium/clustermesh-apiserver:latest | \
numfmt --to=si --suffix=B --format="%.2f"
---> 72.09MB
$ make docker-kvstoremesh-image
$ docker inspect -f '{{ .Size }}' quay.io/cilium/kvstoremesh:latest | \
numfmt --to=si --suffix=B --format="%.2f"
---> 64.62MB
Size of the image after this change:
$ make docker-clustermesh-apiserver-image
$ docker inspect -f '{{ .Size }}' quay.io/cilium/clustermesh-apiserver:latest | \
numfmt --to=si --suffix=B --format="%.2f"
---> 72.26MB
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Align the clustermesh-apiserver Makefile with the strategy already adopted in the ones for the other components, so that it can be included from elsewhere. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
giorio94
force-pushed
the
pr/giorio94/main/clustermesh-apiserver-single-image
branch
from
3f163d0
to
fd489bd
Compare
|
/test |
asauber
approved these changes
Oct 30, 2023
aditighag
approved these changes
Oct 31, 2023
sayboras
approved these changes
Nov 1, 2023
maintainer-s-little-helper
bot
added
the
ready-to-merge
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reporting here the message from the main commit for convenience:
Depends on #27823, #27817, #27873To be discussed in the next community meeting: should we rename the clustermesh-apiserver pod/image to something more expressive which better highlights the new structure (the container is planned to be extended to include the etcd binary, as well as the current init-container logic)? This would require a fairly large amount of CI changes (some of which will likely only pop-up during the next release), as well as make backporting of CI changes more tricky in the future.