Skip to content

Conversation

@imain
Copy link
Contributor

@imain imain commented Oct 4, 2021

Set up a STATIC_IPS configuration flag that configures dev-scripts
to use static IPs (v4 and/or v6) for the baremetal network.

Co-Authored-By: Caleb Boylan [email protected]

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 4, 2021
@openshift-ci
Copy link

openshift-ci bot commented Oct 4, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign hardys after the PR has been reviewed.
You can assign the PR to them by writing /assign @hardys in a comment when ready.

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

Details 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-ci openshift-ci bot requested review from cybertron and sadasu October 4, 2021 21:28
@imain
Copy link
Contributor Author

imain commented Oct 4, 2021

Requires metal3-io/metal3-dev-env#820

Same comment applies here. We could plumb this with a new vm_setup_vars.yaml that sets up a static IP configuration for the network. That may be a better approach. Let me know.

@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 15, 2021
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 18, 2021
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 6, 2021
@openshift-ci openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 6, 2021
@imain imain changed the title WIP: Add ability to inject static ips into install-config Add ability to inject static ips into install-config Dec 13, 2021
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 13, 2021
@bfournie
Copy link
Contributor

/retest

# Note we only do this in the case where METAL3_DEV_ENV is
# unset, to enable developer testing of local checkouts
git reset ee1b2fec44e22700c8d250d5e0e371e3fd0aba17 --hard
#git reset 67afcb3315b44e2c68121493a819819966e588a2 --hard
Copy link

Choose a reason for hiding this comment

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

This needs to be removed - note you can use METAL3_DEV_ENV to reference a local source checkout for testing with latest metal3-dev-env :)

fi

if [[ "$PROVISIONING_NETWORK_PROFILE" == "Disabled" ]]; then
if [[ "$PROVISIONING_NETWORK_PROFILE" == "Disabled" || -n "$STATIC_IPS" ]]; then
Copy link

Choose a reason for hiding this comment

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

I think we should support both virtualmedia and PXE paths now?

bridge: "{{ provisioning_network_name }}"
forward_mode: bridge

virsh_dhcp_enabled: "{{ lookup('env', 'DHCP_ENABLED')|default('', true) }}"
Copy link

Choose a reason for hiding this comment

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

I don't think we need this, since the environment variable is already respected in the defaults ref metal3-io/metal3-dev-env#879 ?

config:
- destination: 0.0.0.0/0
next-hop-address: ${PROVISIONING_HOST_EXTERNAL_IP}
next-hop-interface: ${PROVISIONING_NETWORK_INTERFACE}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We normally would have the default route on the External network, is the change to the provisioning network intentional?

@hardys
Copy link

hardys commented Jan 5, 2022

@imain any updates on this? It'd be useful to get this done so we can attempt reproduces for bugs (for example https://bugzilla.redhat.com/show_bug.cgi?id=2036677) - I think other than the comments already added, we'll also need to inject static configuration for the bootstrap VM?

It'd be great to either create a new periodic CI job too based on this work, or switch the config of one of the existing ones to provide coverage.

@hardys
Copy link

hardys commented Jan 5, 2022

Note there are also a couple of comments on metal3-io/metal3-dev-env#879 which need resolving, then we can merge that and update the dev-scripts pin

netmask_v4: "{{ baremetal_network_cidr_v4|ipaddr('netmask') }}"
address_v6: "{{ baremetal_network_cidr_v6|nthhost(1)|default('', true) }}"
prefix_v6: "{{ baremetal_network_cidr_v6|ipaddr('prefix') }}"
dhcp_enabled: "{{ virsh_dhcp_enabled|bool }}"
Copy link

Choose a reason for hiding this comment

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

Another option is we conditionally define dhcp_range* - then we won't need the new metal3-dev-env flag since the <dhcp stanza is already skipped when the range is omitted

Copy link

Choose a reason for hiding this comment

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

I pushed metal3-io/metal3-dev-env#902 which means we can just set the dhcp_range variables to an empty list, which avoids the need for a new variable

@hardys
Copy link

hardys commented Jan 13, 2022

I pushed #1336 which handles the bootstrap VM ip configuration, which we'll need for this

@derekhiggins derekhiggins mentioned this pull request Jan 26, 2022
Set up a STATIC_IPS configuration flag that configures dev-scripts
to use static IPs (v4 and/or v6) for the baremetal network.

Co-Authored-By: Caleb Boylan <[email protected]>
@openshift-ci openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 10, 2022
@openshift-ci
Copy link

openshift-ci bot commented May 10, 2022

@imain: PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@openshift-ci
Copy link

openshift-ci bot commented Sep 8, 2022

@imain: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-metal-ipi-ovn-dualstack e5d94f1 link false /test e2e-metal-ipi-ovn-dualstack
ci/prow/e2e-metal-ipi-virtualmedia e5d94f1 link false /test e2e-metal-ipi-virtualmedia
ci/prow/e2e-metal-ipi-serial-ovn-ipv6 e5d94f1 link true /test e2e-metal-ipi-serial-ovn-ipv6
ci/prow/e2e-metal-ipi-proxy-ipv4 e5d94f1 link true /test e2e-metal-ipi-proxy-ipv4
ci/prow/e2e-metal-ipi-proxy-ipv6 e5d94f1 link true /test e2e-metal-ipi-proxy-ipv6
ci/prow/e2e-agent-compact e5d94f1 link true /test e2e-agent-compact
ci/prow/e2e-metal-ipi e5d94f1 link true /test e2e-metal-ipi
ci/prow/e2e-metal-ipi-serial-ipv4 e5d94f1 link true /test e2e-metal-ipi-serial-ipv4
ci/prow/e2e-metal-ipi-ovn-ipv6 e5d94f1 link true /test e2e-metal-ipi-ovn-ipv6
ci/prow/e2e-metal-ipi-bm e5d94f1 link true /test e2e-metal-ipi-bm

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@derekhiggins
Copy link
Collaborator

Closing as this has been handled by #1418

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants