Restore the upstream directory structure #200

Merged
mchiappero merged 1 commits from mchiappero/Factory:alignment-directories into main 2025-07-08 09:29:06 +02:00
Owner

Restore the upstream directory structure

It is now possible to bring back the original directory structure for
config (/ironic-config) files and scripts (/scripts). This will make
updates to re-align with upstream easier.

This PR depends on #199

Restore the upstream directory structure It is now possible to bring back the original directory structure for config (/ironic-config) files and scripts (/scripts). This will make updates to re-align with upstream easier. This PR depends on #199
mchiappero added 2 commits 2025-06-30 18:09:22 +02:00
Drop inotify-tools and switch to pyinotify
All checks were successful
Check Release Manifest Local Charts Versions / Check Release Manifest Local Charts Versions (pull_request) Successful in 22s
Build PR in OBS / Build PR in OBS (pull_request_target) Successful in 20s
09c8fb927a
No longer inotifywait use and move to python pyinotify.

See https://github.com/metal3-io/ironic-image/issues/605 for
more details.

Signed-off-by: Marco Chiappero <marco.chiappero@suse.com>
Restore the upstream directory structure
All checks were successful
Check Release Manifest Local Charts Versions / Check Release Manifest Local Charts Versions (pull_request) Successful in 14s
Build PR in OBS / Build PR in OBS (pull_request_target) Successful in -1m15s
b3a46f16f1
It is now possible to bring back the original directory structure for
config (/ironic-config) files and scripts (/scripts). This will make
updates to re-align with upstream easier.

Signed-off-by: Marco Chiappero <marco.chiappero@suse.com>
mchiappero requested review from amorgante 2025-06-30 18:16:53 +02:00
mchiappero requested review from dprodanov 2025-06-30 18:16:55 +02:00
mchiappero force-pushed alignment-directories from b3a46f16f1 to aa9aa54149 2025-07-03 14:50:17 +02:00 Compare
steven.hardy reviewed 2025-07-04 14:44:18 +02:00
@@ -90,1 +89,3 @@
COPY network-data-schema-empty.json /etc/ironic/
COPY ironic-config/ironic.conf.j2 /etc/ironic/
COPY ironic-config/inspector.ipxe.j2 ironic-config/httpd-ironic-api.conf.j2 ironic-config/ipxe_config.template /tmp/
COPY ironic-config/network-data-schema-empty.json /etc/ironic/
Owner

Note this empty schema file doesn't exist upstream, it might be a good candidate for an upstream PR with some variable to enable the related Ironic config (disabled by default upstream) - I think it could be justified as other community members may well be interested in enabling other network-data payloads which don't conform to the openstack schema

Note this empty schema file doesn't exist upstream, it might be a good candidate for an upstream PR with some variable to enable the related Ironic config (disabled by default upstream) - I think it could be justified as other community members may well be interested in enabling other network-data payloads which don't conform to the openstack schema
Author
Owner

So, IIUC, you are proposing to keep it in v30.0.0 as well, rather than removing it as in https://src.opensuse.org/suse-edge/Factory/commit/da9f8f9c2c4cd9cd3ab91ec786af39ae8b7680da3fe?

So, IIUC, you are proposing to keep it in v30.0.0 as well, rather than removing it as in [https://src.opensuse.org/suse-edge/Factory/commit/da9f8f9c2c4cd9cd3ab91ec786af39ae8b7680da3fe](https://src.opensuse.org/suse-edge/Factory/commit/da9f8f9c2c4cd9cd3ab91ec786af39ae8b7680da3fe)?
Owner

Yes it's necessary to keep this, as it allows us to pass nmstate format for the preprovisioningNetworkData and networkData - without the empty schema Ironic won't allow this as it doesn't match the OpenStack network-data format

Yes it's necessary to keep this, as it allows us to pass nmstate format for the preprovisioningNetworkData and networkData - without the empty schema Ironic won't allow this as it doesn't match the OpenStack network-data format
Author
Owner

I see, fundamental to have then... I will rework the changes, once we have a single patch we can create a branch from there and add the missing bits for proposing the change upstream. Thank you!

I see, fundamental to have then... I will rework the changes, once we have a single patch we can create a branch from there and add the missing bits for proposing the change upstream. Thank you!
steven.hardy reviewed 2025-07-04 14:58:33 +02:00
@@ -66,1 +65,3 @@
RUN set -euo pipefail; chmod +x /bin/auth-common.sh; chmod +x /bin/configure-ironic.sh; chmod +x /bin/ironic-common.sh; chmod +x /bin/rundnsmasq; chmod +x /bin/runhttpd; chmod +x /bin/runironic; chmod +x /bin/runlogwatch.sh; chmod +x /bin/tls-common.sh; chmod +x /bin/configure-nonroot.sh;
COPY scripts/ /bin/
COPY configure-nonroot.sh /bin/
RUN set -euo pipefail; chmod +x /bin/configure-ironic.sh /bin/rundnsmasq /bin/runhttpd /bin/runironic /bin/runironic-exporter /bin/runlogwatch.sh /bin/configure-nonroot.sh
Owner

This copies runironic-exporter which AFAICS was not previously copied, and looks like it won't work due to missing dependencies - since we don't currently test/support it I'd suggest we just remove it and skip copying it from upstream until/unless there is a requirement for it?

This copies runironic-exporter which AFAICS was not previously copied, and looks like it won't work due to missing dependencies - since we don't currently test/support it I'd suggest we just remove it and skip copying it from upstream until/unless there is a requirement for it?
Author
Owner

It's just a leftover, I'm not sure how it passed the build...

It's just a leftover, I'm not sure how it passed the build...
Author
Owner

Sorry, I have mis-read your message earlier... So either we copy it to align with upstream until we make a decision or drop it right now? I think it's fine to drop it, but it's not bad to minimize the amount of changes, which is probably what I'd prefer more. Unless I'm missing the point again.

Sorry, I have mis-read your message earlier... So either we copy it to align with upstream until we make a decision or drop it right now? I think it's fine to drop it, but it's not bad to minimize the amount of changes, which is probably what I'd prefer more. Unless I'm missing the point again.
Owner

My point is until now this wasn't copied into the image at all (we had an unused file but it's not COPY'd in the Dockerfile)

With this change it is copied, but AFAICS the script won't work because we don't have gunicorn installed.

So I think we have to either remove it, or make it work, I'm proposing the former as I'm not aware of any use-cases currently which require it, but we can perhaps add the missing dependency if you prefer

My point is until now this wasn't copied into the image at all (we had an unused file but it's not COPY'd in the Dockerfile) With this change it is copied, but AFAICS the [script won't work]([url](https://src.opensuse.org/suse-edge/Factory/src/branch/main/ironic-image/runironic-exporter#L11)) because we don't have gunicorn installed. So I think we have to either remove it, or make it work, I'm proposing the former as I'm not aware of any use-cases currently which require it, but we can perhaps add the missing dependency if you prefer
Author
Owner

Thanks for the input, see PR #204.

Thanks for the input, see PR #204.
nbelouin approved these changes 2025-07-07 09:17:31 +02:00
Dismissed
nbelouin left a comment
Owner

LGTM, as far as I can see all comments were addressed and I don't see any other issue here, so 👍

LGTM, as far as I can see all comments were addressed and I don't see any other issue here, so 👍
mchiappero force-pushed alignment-directories from aa9aa54149 to ef6989b0d8 2025-07-07 23:47:11 +02:00 Compare
nbelouin approved these changes 2025-07-08 09:28:17 +02:00
mchiappero merged commit ef6989b0d8 into main 2025-07-08 09:29:06 +02:00
mchiappero deleted branch alignment-directories 2025-07-08 09:29:08 +02:00
Sign in to join this conversation.
No Label
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: suse-edge/Factory#200
No description provided.