-
Notifications
You must be signed in to change notification settings - Fork 23
Add os_purpose to all images. #952
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
Conversation
Most are generic, the Ubuntu -minimal versions and cirros are minimal, opnsense is custom and flatcar, kubernetes and gardenlinux are capinode. Signed-off-by: Kurt Garloff <[email protected]>
|
Comments welcome, @berendt, @mbuechse, @horazont, @depressiveRobot. |
Signed-off-by: Kurt Garloff <[email protected]>
|
Hmm, test fail (flake8?!) looks bogus to me: |
This addresses issue osism#953. Notes: - Used version 20250903-2224 - Imaging on OSISM swift still needed - I already included the os_purpose field (see PR osism#952), so this will lead to a schema validation error until we merge osism#952. In case we decide to merge this one first, we can of course easily drop one line. Signed-off-by: Kurt Garloff <[email protected]>
|
I'm wondering whether the code needs to be adapted as well. See this line (and the following): https://github.com/osism/openstack-image-manager/blob/main/openstack_image_manager/main.py#L361 Something like this should probably be added for the new field as well. |
I don't think so. |
|
Sorry, you're right of course. I only faintly remembered (from earlier encounters with the code) that some kind of copying was being done in the script, namely in https://github.com/osism/openstack-image-manager/blob/main/openstack_image_manager/main.py#L298 and following, but first of all, that doesn't seem to cause a problem here, and second of all, the line that I referenced in the earlier comment was definitely doing what you now said. Sorry for the confusion! |
Signed-off-by: Kurt Garloff <[email protected]>
|
OK, ready from my perspective. |
|
Well, lgtm, but let's hear what @berendt has to say. |
|
@berendt, any reason not to proceed? |
|
To be frank, v1.1 is already merged. However, we want to do v2 very soon where this field will be REQUIRED. |
|
Done. Please specify in future if the merge should be completed by a certain date. |
|
Thanks, Christian. It was not exactly urgent, just wanted to ensure there is progress in time before SCS R9 next week. |
The `os_purpose` field was introduced in [1]. [1] osism/openstack-image-manager#952 Signed-off-by: Jan Horstmann <[email protected]>
The `os_purpose` field was introduced in [1]. [1] osism/openstack-image-manager#952 Signed-off-by: Jan Horstmann <[email protected]>
The `os_purpose` field was introduced in [1]. [1] osism/openstack-image-manager#952 Signed-off-by: Jan Horstmann <[email protected]>
Most are generic, the Ubuntu -minimal versions and cirros are minimal, opnsense is custom and flatcar, kubernetes, talos and gardenlinux are capinode.
This implements the changes suggested in PR SCS Standards #970.
Notes:
network. I had removed it after a discussion, as the intention was to not have categories that we are not using. But we would actually usenetwork, so I'm in favor of adding it back.capinodeimage is slightly misleading, as Gardener does not use Cluster-API (which is where capi comes from), though the concept here is still the same AFAIK. I'm neutral as to whether we introduce a categoryk8snodeinstead ofcapinode.minimalorcapinode(ork8snode).