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

packaging: Don't always build the kata-agent #8916

Conversation

fidencio
Copy link
Member

There's absolutely no need to always build the Kata Containers agent when building the rootfs. We already have the agent being built / packaged on every PR, and we can rely on that to simply be unpacked into the rootfs, unless there's a change in the agent.

Please, see each commit for more info.

@katacontainersbot katacontainersbot added the size/medium Average sized task label Jan 25, 2024
@katacontainersbot katacontainersbot added size/large Task of significant size and removed size/medium Average sized task labels Jan 25, 2024
@fidencio fidencio force-pushed the topic/packaging-reuse-already-built-agent branch 3 times, most recently from f855b01 to 5f9f531 Compare January 25, 2024 12:31
This has been missed during reviews and will become a problem when the
tools start to be built in different architectures.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
This has been missed during reviews and is already a problem as we're
trying to build the agent outside of the rootfs for other architectures
than x86_64.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the update - I hope this will save a lots of CI time and money!

@fidencio fidencio force-pushed the topic/packaging-reuse-already-built-agent branch 4 times, most recently from cfa4a01 to a22b50b Compare January 25, 2024 17:49
We're moving away from alpine and using ubuntu in order to be able to
build the agent for all the architectures we need.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
This env var will serve us to pass the agent tarball to the rootfs
builder, which will then just unpack the content into the rootfs instead
of building the agent again.

AGENT_TARBALL and AGENT_SOURCE_BIN should never be used together.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
As we'll be untarring the agent tarball (and any other component that
may be part of the rootfs) into the rootfs, we have to have xz
installed.

For debian and ubuntu the package is called xz-utils; for centos,
alpine and cbl-mariner the package is called xz.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Let's always do this, regardless of where the agent is coming from.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
It just means the component is not cached, and that it must be built in
the usual way.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Let's start relying on the already cached agent to be deployed inside
the rootfs.  By doing this we save a lot of time in our CI, and we have
a better way, for developers, to play with changes in the agent.

Fixes: kata-containers#8915

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
@fidencio fidencio force-pushed the topic/packaging-reuse-already-built-agent branch from a22b50b to dd49479 Compare January 25, 2024 18:41
@fidencio
Copy link
Member Author

/test

@danmihai1
Copy link
Contributor

A lot of goodness here - thanks @fidencio !

@@ -693,6 +692,8 @@ EOF
tar xvJpf ${AGENT_TARBALL} -C ${ROOTFS_DIR}
fi

${stripping_tool} ${ROOTFS_DIR}/usr/bin/kata-agent
Copy link
Contributor

Choose a reason for hiding this comment

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

We can do this later, but perhaps we should have a way to bypass this new behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd prefer doing this later, but just to confirm.

The idea here is that we can bypass the behaviour and actually not have the component installed, correct?
If so, I think we can enforce this for the attestation-agent / confiential-data-hub / opa, are those are extra components. The kata agent itself must always be there, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Never mind. Yesterday I thought this rootfs.sh change was a significant change in symbol strip behavior - but I see now that it's consistent with the previous behavior.

Otherwise building as root will not work, as demonstrated by the arm64
CI.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
@fidencio
Copy link
Member Author

/test

@stevenhorsman stevenhorsman self-requested a review January 26, 2024 09:50
Copy link
Member

@stevenhorsman stevenhorsman left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks!

@fidencio fidencio marked this pull request as ready for review January 26, 2024 10:08
@fidencio fidencio merged commit a7c6822 into kata-containers:main Jan 26, 2024
282 of 288 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ok-to-test size/large Task of significant size
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants