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

[os_net_setup] Add --availability-zone-hint to network creation #2497

Conversation

eduolivares
Copy link
Contributor

This PR adds the option to configure external networks for certain AZs. In order to do this, the list availability_zone_hints should be configured as port of cifmw_os_net_setup_config.

@github-actions github-actions bot marked this pull request as draft October 29, 2024 12:27
Copy link

Thanks for the PR! ❤️
I'm marking it as a draft, once your happy with it merging and the PR is passing CI, click the "Ready for review" button below.

@pablintino
Copy link
Collaborator

/approve

Copy link
Contributor

openshift-ci bot commented Oct 30, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pablintino

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

@pablintino pablintino requested a review from a team October 30, 2024 13:30
@evallesp
Copy link

(non-blocking) suggestion: I see some vars used in the template that are missing of some context and format.
Unsure what's the format of availability_zone_hints but I might have a lot of missing context.

WDYT about adding that to the default file: https://github.com/openstack-k8s-operators/ci-framework/blob/main/roles/os_net_setup/defaults/main.yml to see an example?

I'm thinking if it might make sense to rework eventually in the README file to briefly explain each of the possibilities of: cifmw_os_net_setup_config

Apart of this: lgtm

Copy link
Collaborator

@lewisdenny lewisdenny left a comment

Choose a reason for hiding this comment

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

I agree with @evallesp

Please add availability_zone_hints to the role defaults, others are missing but they can be added in a follow up if we get time to refactor this role.

@lewisdenny lewisdenny requested review from a team and removed request for rebtoor October 31, 2024 04:56
This PR adds the option to configure external networks for certain AZs.
In order to do this, the list `availability_zone_hints` should be
configured as port of `cifmw_os_net_setup_config`.
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/de855c24e2634b82be544cbafc270baa

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 57m 02s
podified-multinode-edpm-deployment-crc FAILURE in 40m 41s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 28m 17s
✔️ noop SUCCESS in 0s
cifmw-pod-ansible-test NODE_FAILURE Node request 100-0007647506 failed in 0s
cifmw-pod-pre-commit NODE_FAILURE Node request 100-0007647507 failed in 0s
✔️ build-push-container-cifmw-client SUCCESS in 36m 45s
✔️ cifmw-molecule-os_net_setup SUCCESS in 3m 57s

@eduolivares
Copy link
Contributor Author

recheck

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/1a95eda3d3d44e2b8d13236505d0df66

✔️ openstack-k8s-operators-content-provider SUCCESS in 1h 49m 08s
✔️ podified-multinode-edpm-deployment-crc SUCCESS in 1h 19m 50s
✔️ cifmw-crc-podified-edpm-baremetal SUCCESS in 1h 33m 43s
✔️ noop SUCCESS in 0s
cifmw-pod-ansible-test NODE_FAILURE Node request 100-0007647714 failed in 0s
cifmw-pod-pre-commit NODE_FAILURE Node request 100-0007647715 failed in 0s
✔️ build-push-container-cifmw-client SUCCESS in 37m 21s
✔️ cifmw-molecule-os_net_setup SUCCESS in 4m 22s

@pablintino
Copy link
Collaborator

recheck

Copy link
Collaborator

@lewisdenny lewisdenny left a comment

Choose a reason for hiding this comment

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

/lgtm

Thanks @eduolivares ++

@openshift-ci openshift-ci bot added the lgtm label Oct 31, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 1fcdd76 into openstack-k8s-operators:main Oct 31, 2024
4 checks passed
@eduolivares eduolivares deleted the external-network-az branch November 5, 2024 16:37
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.

4 participants