Skip to content

remove /etc/systemd/system/uyuni-server.service.d/Service.conf during uninstallation #293

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

Merged
merged 3 commits into from
May 15, 2024

Conversation

mbussolotto
Copy link
Member

What does this PR change?

See the title

Test coverage

  • DONE

Links

Issue(s): #

  • DONE

Changelogs

Make sure the changelogs entries you are adding are compliant with https://github.com/uyuni-project/uyuni/wiki/Contributing#changelogs and https://github.com/uyuni-project/uyuni/wiki/Contributing#uyuni-projectuyuni-repository

If you don't need a changelog check, please mark this checkbox:

  • No changelog needed

If you uncheck the checkbox after the PR is created, you will need to re-run changelog_test (see below)

Before you merge

Check How to branch and merge properly!

@mbussolotto mbussolotto requested a review from cbosdo May 6, 2024 10:24
Copy link
Contributor

@cbosdo cbosdo left a comment

Choose a reason for hiding this comment

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

I'm fine with this PR, but where do we create that file? Shouldn't we also remove the other config files there too?

@@ -39,16 +39,23 @@ func GetServicePath(name string) string {
return path.Join(servicesPath, name+".service")
}

// GetServiceConfPath return the path for Service.conf file.
func GetServiceConfPath(name string) string {
return path.Join(servicesPath, name+".service.d", "Service.conf")
Copy link
Contributor

Choose a reason for hiding this comment

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

It is the only file we want to remove in that folder? Shouldn't we remove all user-added files there too?

Copy link
Member Author

Choose a reason for hiding this comment

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

For sure the folder should be removed if there're no files, I'm going to add this change.
I'm not sure if we should remove the user-added file: I think we should have the same behaviour of command like zypper uninstall and I think the user file are not deleted by that command.
What do you think?

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 leave them, but a new install after would be polluted by those... not sure what's best.

@mbussolotto
Copy link
Member Author

I'm fine with this PR, but where do we create that file? Shouldn't we also remove the other config files there too?

Service.conf is the only file created by mgradm (besides uyuni-server.service and uyuni-server-attestation.service in coco attestation container) . uyuni-server.service.d might contains other files created by the user...should we remove them during uinstallation?

@mbussolotto mbussolotto force-pushed the remove_service_conf branch from 0338d9e to 7a9261a Compare May 13, 2024 09:44
@mbussolotto mbussolotto requested a review from cbosdo May 13, 2024 13:13
@mbussolotto
Copy link
Member Author

@cbosdo Check if this might be a good option:

  • folder is removed if it's empty
  • a warning message is printed if folder contains user files

@mbussolotto mbussolotto merged commit 087b0f4 into uyuni-project:main May 15, 2024
6 checks passed
mbussolotto added a commit to mbussolotto/uyuni-tools that referenced this pull request May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants