suse-edge/Factory
SHA256
13
0
Fork 14
Code Issues Pull Requests 6 Actions Packages Projects Releases Wiki Activity

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
Conversation 4 Commits 1 Files Changed 20 +11 -10
mchiappero commented 2025-06-30 18:09:20 +02:00
Owner
Copy Link

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
Details
Build PR in OBS / Build PR in OBS (pull_request_target) Successful in 20s
Details
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
Details
Build PR in OBS / Build PR in OBS (pull_request_target) Successful in -1m15s
Details
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
ironic-image/Dockerfile Outdated
@@ -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/
steven.hardy commented 2025-07-04 14:44:18 +02:00
Owner
Copy Link

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
mchiappero commented 2025-07-04 14:48:51 +02:00
Author
Owner
Copy Link

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)?
steven.hardy commented 2025-07-04 15:01:23 +02:00
Owner
Copy Link

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
mchiappero commented 2025-07-04 16:11:14 +02:00
Author
Owner
Copy Link

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!
👍 1
steven.hardy reviewed 2025-07-04 14:58:33 +02:00
ironic-image/Dockerfile Outdated
@@ -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
steven.hardy commented 2025-07-04 14:58:33 +02:00
Owner
Copy Link

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?
mchiappero commented 2025-07-04 16:12:56 +02:00
Author
Owner
Copy Link

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...
mchiappero commented 2025-07-04 16:25:47 +02:00
Author
Owner
Copy Link

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.
steven.hardy commented 2025-07-04 16:45:14 +02:00
Owner
Copy Link

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
mchiappero commented 2025-07-04 17:32:28 +02:00
Author
Owner
Copy Link

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
Copy Link

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.
Reviewers
No Reviewers
amorgante
dprodanov
nbelouin
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
amorgante atanasdinov atrendafilov dbekhit dprodanov eminguez fdegirmenci ipetrov117 jtomasek kzhelyazkov mchiappero nbelouin roxenham steven.hardy
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: suse-edge/Factory#200
No description provided.
Delete Branch "mchiappero/Factory:alignment-directories"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?