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

Bug: fix the aa-kbc-params error in fedora #1853

Closed
wants to merge 5 commits into from

Conversation

huoqifeng
Copy link

Fixes: #1852

Fixes: confidential-containers#1852

Signed-off-by: Qi Feng Huo <huoqif@cn.ibm.com>
@huoqifeng
Copy link
Author

@mkulke @bpradipt not sure whether it's reasonable for azure and aws provider, may you please check?

@huoqifeng
Copy link
Author

huoqifeng commented Jun 6, 2024

The problem is on fedora, the failure in ExecStartPre in process-user-data blocks ExecStart for libvirt provider.
I broken it into 2 services. process-user-data-provision and process-user-data-update while process-user-data-update depends on cloud-final.service because libvirt and other providers like ibmcloud uses cloud-init to provision user-data.

@huoqifeng huoqifeng requested a review from liudalibj June 6, 2024 06:08
Fixes: confidential-containers#1852

Signed-off-by: Qi Feng Huo <huoqif@cn.ibm.com>
@huoqifeng huoqifeng added the test_e2e_libvirt Run Libvirt e2e tests label Jun 6, 2024
@bpradipt
Copy link
Member

bpradipt commented Jun 6, 2024

@huoqifeng how was the libvirt provider working all this while with the packer created image?

@@ -19,7 +19,7 @@ const (
providerAzure = "azure"
providerAws = "aws"

defaultAgentConfigPath = "/etc/agent-config.toml"
defaultAgentConfigPath = "/run/peerpod/agent-config.toml"
Copy link
Member

Choose a reason for hiding this comment

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

the defaultAgentConfigPath is changed here but
src/cloud-api-adaptor/podvm/files/etc/systemd/system/kata-agent.service didn't use the new changed path.

Copy link
Author

Choose a reason for hiding this comment

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

@liudalibj thanks for finding it, I think that's another reason why it works on ubuntu but not on fedora, fedora altered the path to /run/peerpod/agent-config.toml here https://github.com/confidential-containers/cloud-api-adaptor/blob/main/src/cloud-api-adaptor/podvm-mkosi/mkosi.skeleton/usr/lib/systemd/system/kata-agent.service.d/10-override.conf

Copy link
Author

Choose a reason for hiding this comment

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

I guess we should change to use /run/peerpod/agent-config.toml on ubuntu also.

Copy link
Author

Choose a reason for hiding this comment

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

I'll give a try after update the path...

Copy link
Member

Choose a reason for hiding this comment

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

Fixes: confidential-containers#1852

Signed-off-by: Qi Feng Huo <huoqif@cn.ibm.com>
@huoqifeng
Copy link
Author

@huoqifeng how was the libvirt provider working all this while with the packer created image?

I'm not 100% sure, but I guess ExecStartPre failure won't cause ExecStart skipping on ubuntu.
Another thing I'm digging is why after the service change I made in this PR. the file /etc/agent-config.toml changed but not /run/peerpod/agent-config.toml without the main.go change. -- It worked on ubuntu image. @bpradipt

Qi Feng Huo added 2 commits June 6, 2024 14:55
Fixes: confidential-containers#1852

Signed-off-by: Qi Feng Huo <huoqif@cn.ibm.com>
Fixes: confidential-containers#1852

Signed-off-by: Qi Feng Huo <huoqif@cn.ibm.com>
@@ -3,7 +3,7 @@ Distribution=fedora

[Distribution]
Distribution=fedora
Release=38
Release=39
Copy link
Collaborator

Choose a reason for hiding this comment

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

if we want to bump fedora, can we do this in a separate PR?

Wants=process-user-data.service
After=netns@podns.service process-user-data.service
Wants=process-user-data-update.service
After=netns@podns.service process-user-data-update.service
Copy link
Collaborator

Choose a reason for hiding this comment

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

regarding the idea discussed in #1852 (comment), could we instead change the logic to provision the agent-config.file via cloud-config entry, so we don't have to perform any "updates"?

@huoqifeng
Copy link
Author

huoqifeng commented Jun 12, 2024

I guess this has been fixed by PR #1858 @mkulke

@mkulke
Copy link
Collaborator

mkulke commented Jun 12, 2024

I guess this has been fixed by PR #1858 @mkulke

I wouldn't expect this (at least intentionally). the above PR should not change behaviour in this regard.

@huoqifeng
Copy link
Author

Close it as it has been fixed in #1868

@huoqifeng huoqifeng closed this Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test_e2e_libvirt Run Libvirt e2e tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

aa-kbc-params is not customized in agent-config.toml for libvirt provider in fedora
4 participants