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

do-not-merge: testing the handler for containerd <= 1.6 #276

Closed
wants to merge 7 commits into from

Conversation

wainersm
Copy link
Member

No description provided.

fidencio and others added 6 commits October 27, 2023 14:55
Right now we're just ignoring the global env vars passed, which is not
the expected behaviour, mainly as we're duplicating a bunch of env vars
passed to the preInstall / postUninstall payloads.

Let's ensure we actually read the global ones and take those into
consideration as well.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Otherwise we may end up in situations where users will simply try to add
/ replace an environment variable via kustomize and will end up with the
operator broken as this env var would simply be vanished. :-/

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
Otherwise we may hit issues in in cases where folks will use kustomize
to add / replace one the values, as kustomize just overrides the whole
set of env vars.

We need to keep those in sync with whatever is set in the configs, but
this shouldn't be as much of a burden as leaving it for the user to do
so.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
In theory those could very well be set only for the pre-install /
post-uninstall, but that doesn't make much sense as the user experience
on setting those would be to having to set those twice.

With the user experience in mind, let's just move them to the global env
vars and let the user set / kustomize them only once.

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
The majority of the managed kubernetes solutions are already relying on
a new enough (v1.7+) version of containerd.

Fixes: confidential-containers#272

Signed-off-by: Fabiano Fidêncio <fabiano.fidencio@intel.com>
With the new requirement of containerd 1.7+, the operator.sh was adapted
to enable the operator to install containerd in case the system's
installed version is less than or equal to 1.6.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
@wainersm
Copy link
Member Author

/test-kata-qemu

@wainersm
Copy link
Member Author

/test-kata-clh

1 similar comment
@wainersm
Copy link
Member Author

/test-kata-clh

@wainersm
Copy link
Member Author

/test-kata-qemu

@wainersm
Copy link
Member Author

tdx test is passed: http://10.112.240.228:8080/job/confidential-containers-operator-main-centos8stream-x86_64-containerd_kata-qemu-tdx-PR/589/

cool, thanks for letting me know @ChengyuZhu6 !

I'm debugging why it fails to start a container registry on Ubuntu 22.04; locally I can make it work just fine :(

On Ubuntu 22.04 installs docker from the distro and consequently
containerd version 1.7. This ways we cover the default installation case where
INSTALL_OFFICIAL_CONTAINERD=false.

Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
@wainersm
Copy link
Member Author

/test-kata-qemu

@wainersm
Copy link
Member Author

/test-kata-clh

@fidencio
Copy link
Member

fidencio commented Nov 3, 2023

Closing as #274 has the patches provided here.
Thanks @wainersm!

@fidencio fidencio closed this Nov 3, 2023
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

3 participants