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

Prepare networking as early as possible #1717

Merged
merged 1 commit into from
May 28, 2024

Conversation

cjeanner
Copy link
Contributor

@cjeanner cjeanner commented May 16, 2024

In order to change how we provision networking, especially DHCP, we have
to get the virtual networks as early as possible in place.

Doing so, we'll be able to inject some specific VM, such as the nat64,
and properly manage networks, even for devscripts layout.

As a pull request owner and reviewers, I checked that:

  • Appropriate testing is done and actually running

@cjeanner testing notes:

  • CRC layout is passing fine, networks are present and working as expected
  • 3-OCP layout is passing fine as well, networks are present and working as expected

Copy link
Contributor

openshift-ci bot commented May 16, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

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://review.rdoproject.org/zuul/buildset/a9703ac4e4e94d57940babeb220dcd62

✔️ noop SUCCESS in 0s
✔️ cifmw-pod-ansible-test SUCCESS in 9m 38s
✔️ cifmw-pod-pre-commit SUCCESS in 8m 06s
cifmw-molecule-devscripts FAILURE in 7m 59s
cifmw-molecule-libvirt_manager FAILURE in 12m 09s
cifmw-molecule-reproducer FAILURE in 4m 27s

@cjeanner cjeanner force-pushed the reproducer/pre-networking branch from 958ed49 to 1db4ec9 Compare May 22, 2024 13:38
@cjeanner cjeanner marked this pull request as ready for review May 22, 2024 15:12
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.

If I understand correctly the tasks that we're copied from roles/devscripts/tasks/133_host_network.yml into roles/reproducer/tasks/prepare_networking.yml were protected by a when clause:

    - cifmw_devscripts_config.manage_br_bridge is defined
    - cifmw_devscripts_config.manage_br_bridge == 'n'

Is this no longer needed or being applied somewhere else?

@cjeanner
Copy link
Contributor Author

If I understand correctly the tasks that we're copied from roles/devscripts/tasks/133_host_network.yml into roles/reproducer/tasks/prepare_networking.yml were protected by a when clause:

    - cifmw_devscripts_config.manage_br_bridge is defined
    - cifmw_devscripts_config.manage_br_bridge == 'n'

Is this no longer needed or being applied somewhere else?

It was never really tested in molecule, and I happened to trigger that path by passing that specific override in the molecule test, leading to.... well.. "some" issues with that untested track, mostly due to lacking data that are usually fed by a job definition.... So I changed the override to keep it simple in the other PR.

In order to change how we provision networking, especially DHCP, we have
to get the virtual networks as early as possible in place.

Doing so, we'll be able to inject some specific VM, such as the nat64,
and properly manage networks, even for devscripts layout.
@raukadah
Copy link
Contributor

/approve
thank you for testing on both crc and devscripts.

Copy link
Contributor

openshift-ci bot commented May 28, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: raukadah

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
Copy link
Collaborator

/lgtm

@openshift-ci openshift-ci bot added the lgtm label May 28, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit 5f4d7bf into main May 28, 2024
5 checks passed
@openshift-merge-bot openshift-merge-bot bot deleted the reproducer/pre-networking branch May 28, 2024 10:39
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.

5 participants