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

Document nmstate as experimental and require an override to use it #843

Merged

Conversation

slagle
Copy link
Contributor

@slagle slagle commented Dec 9, 2024

"edpm_network_config_tool: nmstate" is experimental and officially
unsupported. This change will require users to explicitly opt-in to
using nmstate directly instead of using os-net-config+nmstate. It adds a
task that will fail unless edpm_network_config_tool_nmstate_override is
set to true (defaults to false).

Signed-off-by: James Slagle [email protected]
Jira: https://issues.redhat.com/browse/OSPRH-12316

Copy link

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://softwarefactory-project.io/zuul/t/rdoproject.org/buildset/6de467403e2c4c3e9b69d53848c75034

✔️ openstack-k8s-operators-content-provider SUCCESS in 3h 29m 45s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 19m 36s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 33m 47s
✔️ edpm-ansible-tempest-multinode SUCCESS in 1h 54m 35s
edpm-ansible-molecule-edpm_bootstrap FAILURE in 4m 11s
✔️ edpm-ansible-molecule-edpm_podman SUCCESS in 4m 58s
✔️ edpm-ansible-molecule-edpm_module_load SUCCESS in 4m 01s
✔️ edpm-ansible-molecule-edpm_kernel SUCCESS in 7m 30s
edpm-ansible-molecule-edpm_libvirt FAILURE in 4m 07s
edpm-ansible-molecule-edpm_nova FAILURE in 4m 05s
edpm-ansible-molecule-edpm_frr FAILURE in 3m 59s
✔️ edpm-ansible-molecule-edpm_iscsid SUCCESS in 4m 11s
edpm-ansible-molecule-edpm_ovn_bgp_agent FAILURE in 4m 00s
edpm-ansible-molecule-edpm_ovs FAILURE in 4m 36s
✔️ edpm-ansible-molecule-edpm_tripleo_cleanup SUCCESS in 3m 33s
edpm-ansible-molecule-edpm_tuned FAILURE in 4m 15s
edpm-ansible-molecule-edpm_telemetry_power_monitoring FAILURE in 4m 37s
edpm-ansible-molecule-edpm_update FAILURE in 3m 56s
✔️ adoption-standalone-to-crc-ceph-provider SUCCESS in 3h 08m 45s

Copy link
Contributor

@bshephar bshephar left a comment

Choose a reason for hiding this comment

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

Do you think more things like this are likely to come up in the future?

I wonder if a better approach might be to have a dictionary called something like edpm_tech_preview. Then allow users to enable tech preview features be appending them to the list?

Obviously, this isn't required if we're only doing it for nmstate. But I do wonder how many more of these things will come up in the future and how cumbersome it will become to individually manage them on a per role basis?

Similar to feature gates I guess:
https://docs.openshift.com/container-platform/4.17/nodes/clusters/nodes-cluster-enabling-features.html

https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/

Could be overkill for our requirements, but putting it out there for consideration.

@slagle
Copy link
Contributor Author

slagle commented Dec 10, 2024

Do you think more things like this are likely to come up in the future?

I wonder if a better approach might be to have a dictionary called something like edpm_tech_preview. Then allow users to enable tech preview features be appending them to the list?

Obviously, this isn't required if we're only doing it for nmstate. But I do wonder how many more of these things will come up in the future and how cumbersome it will become to individually manage them on a per role basis?

Similar to feature gates I guess: https://docs.openshift.com/container-platform/4.17/nodes/clusters/nodes-cluster-enabling-features.html

https://kubernetes.io/docs/reference/command-line-tools-reference/feature-gates/

Could be overkill for our requirements, but putting it out there for consideration.

Yes, I could see adding a field like this on the NodeSet, which would set an ansible var to enable certain features. I think it would consider it separately from this PR

@slagle slagle requested review from vcandapp and removed request for viroel and frenzyfriday December 10, 2024 16:55
"edpm_network_config_tool: nmstate" is experimental and officially
unsupported. This change will require users to explicitly opt-in to
using nmstate directly instead of using os-net-config+nmstate. It adds a
task that will fail unless edpm_network_config_tool_nmstate_override is
set to true (defaults to false).

Signed-off-by: James Slagle <[email protected]>
Jira: https://issues.redhat.com/browse/OSPRH-12316
Copy link
Contributor

openshift-ci bot commented Dec 11, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bshephar, slagle

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit c18c898 into openstack-k8s-operators:main Dec 11, 2024
34 checks passed
@vcandapp
Copy link
Contributor

ltgm

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.

3 participants