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

Add foreman_ygg_migration package #11317

Open
wants to merge 3 commits into
base: rpm/develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions comps/comps-foreman-client-el8.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<uservisible>true</uservisible>
<packagelist>
<packagereq type="default">foreman-client-release</packagereq>
<packagereq type="default">foreman_ygg_migration</packagereq>
<packagereq type="default">foreman_ygg_worker</packagereq>
<packagereq type="default">katello-host-tools</packagereq>
<packagereq type="default">katello-host-tools-tracer</packagereq>
Expand Down
1 change: 1 addition & 0 deletions comps/comps-foreman-client-el9.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
<uservisible>true</uservisible>
<packagelist>
<packagereq type="default">foreman-client-release</packagereq>
<packagereq type="default">foreman_ygg_migration</packagereq>
<packagereq type="default">foreman_ygg_worker</packagereq>
<packagereq type="default">katello-host-tools</packagereq>
<packagereq type="default">katello-host-tools-tracer</packagereq>
Expand Down
1 change: 1 addition & 0 deletions package_manifest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,7 @@ foreman_client_packages:
children:
yggdrasil_client_packages: {}
hosts:
foreman_ygg_migration: {}
foreman_ygg_worker: {}
katello-host-tools: {}
katello-pull-transport-migrate: {}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
Name: foreman_ygg_migration
Copy link
Member

Choose a reason for hiding this comment

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

I am debating with myself whether this needs to be an own package, vs a sub-package of foreman_ygg_worker (smth like foreman_ygg_worker_migration or so).

Copy link
Author

Choose a reason for hiding this comment

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

That would be cleaner, but at the same time it is not a migration of foreman_ygg_worker

Version: 0.0.1
Summary: A helper package to ease transition from yggdrasil 0.2.z to 0.4.z
Release: 1%{?dist}
License: MIT

%if 0%{?rhel} >= 8
Supplements: yggdrasil >= 0.4.0
Copy link
Member

Choose a reason for hiding this comment

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

People can (and do 😢) disable weak dependencies.
For such setups it will mean that this package won't be automatically installed once they upgrade to 0.4.

This is not a blocker (and there is no better package relationship to fix this), but something we should keep in mind and document somewhere.

Copy link
Member

Choose a reason for hiding this comment

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

What exactly should we document and where?

Copy link
Author

Choose a reason for hiding this comment

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

We should document that if people have weak dependencies disabled, they should install this package explicitly. As to where, I'm not sure if we already have a procedure or something where this would fit well or we'll need to write a new one

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I am not aware of a good place for this (maybe at least release notes?)

%endif
Conflicts: yggdrasil < 0.4.0

%description
A helper package to ease transition from yggdrasil 0.2.z to 0.4.z

%post
if systemctl is-enabled yggdrasild >/dev/null 2>/dev/null; then
grep -Pq '^server' /etc/yggdrasil/config.toml || sed -i 's/broker.*=/server =/' /etc/yggdrasil/config.toml
grep -Pq '^path-prefix' /etc/yggdrasil/config.toml || echo 'path-prefix = "yggdrasil"' >>/etc/yggdrasil/config.toml
grep -Pq '^data-host' /etc/yggdrasil/config.toml || echo 'data-host = ""' >>/etc/yggdrasil/config.toml

if grep -q FOREMAN_YGG_WORKER_WORKDIR /etc/systemd/system/yggdrasild.service.d/* 2>/dev/null; then
mkdir -p /etc/systemd/system/com.redhat.Yggdrasil1.Worker1.foreman.service.d/
cp -r /etc/systemd/system/yggdrasild.service.d/* /etc/systemd/system/com.redhat.Yggdrasil1.Worker1.foreman.service.d/
Copy link
Member

Choose a reason for hiding this comment

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

After this, you may need to run systemctl daemon-reload.

systemctl daemon-reload
fi

systemctl disable --now yggdrasild
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't the old service get uninstalled and thus removed?

Copy link
Author

Choose a reason for hiding this comment

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

From what I've seen, we're missing systemd scriptlets in our yggdrasil spec, so while the old package gets uninstalled and the yggdrasild service definition goes away, it seems to stay enabled.

systemctl enable --now yggdrasil
Copy link
Member

Choose a reason for hiding this comment

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

It may need to restart yggdrasil if it's already running.

Copy link
Author

Choose a reason for hiding this comment

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

That would mean that someone installed new-enough yggdrasil, started it and then installed the migration package, correct?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, though I also wonder if that use case would be one to error out on. And you really shouldn't error out on RPM scriptlets. Still not entirely sure what's best here and currently my mind is exploring the various paths a user could take.

fi

%files

%changelog
* Thu Oct 03 2024 Adam Ruzicka <[email protected]> - 0.0.1-1
- Initial packaging