MGMT-18513: Add machine networks validation when iSCSI boot disk is in use by adriengentil · Pull Request #6978 · openshift/assisted-service

adriengentil · 2024-11-08T12:10:34Z

In order to successfully install an OCP cluster using iSCSI boot disk,
we need to ensure that the machine networks doesn't belong the IP
addresses set on the network interface connected to the iSCSI
disk.

In a previous effort
(#6434), we ensured
that the iSCSI disk can be selected as installation disk only if the
machine has 2 network interfaces.

Example with SNO (same behavior with MNO+CMN), the iSCSI disk is connected through the subnet 192.168.11.0/24:

openshift-ci · 2024-11-08T12:10:38Z

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

openshift-ci · 2024-11-08T12:11:00Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adriengentil

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [adriengentil]

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

openshift-ci-robot · 2024-11-08T12:12:03Z

@adriengentil: This pull request references MGMT-18513 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.

Details

In response to this:

MGMT-18513: machine networks must not be part of iSCSI subnet

MGMT-18513: Add machine networks validation when iSCSI boot disk is in use

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 openshift-eng/jira-lifecycle-plugin repository.

openshift-ci-robot · 2024-11-08T12:12:19Z

@adriengentil: This pull request references MGMT-18513 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.

Details

In response to this:

In order to successfully install an OCP cluster using iSCSI boot disk,
we need to ensure that the machine networks doesn't belong the IP
addresses set on the network interface connected to the iSCSI
disk.

In a previous effort
(#6434), we ensured
that the iSCSI disk can be selected as installation disk only if the
machine has 2 network interfaces.

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 openshift-eng/jira-lifecycle-plugin repository.

adriengentil · 2024-11-08T12:12:31Z

/test edge-e2e-metal-assisted

adriengentil · 2024-11-08T13:57:24Z

/test edge-lint edge-unit-test

openshift-ci-robot · 2024-11-08T14:22:49Z

@adriengentil: This pull request references MGMT-18513 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target the "4.18.0" version, but no target version was set.

Details

In response to this:

In order to successfully install an OCP cluster using iSCSI boot disk,
we need to ensure that the machine networks doesn't belong the IP
addresses set on the network interface connected to the iSCSI
disk.

In a previous effort
(#6434), we ensured
that the iSCSI disk can be selected as installation disk only if the
machine has 2 network interfaces.

Example with SNO (same behavior with MNO+CMN), the iSCSI disk is connected through the subnet 192.168.11.0/24:

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 openshift-eng/jira-lifecycle-plugin repository.

adriengentil · 2024-11-08T14:25:15Z

/test edge-e2e-metal-assisted

codecov · 2024-11-08T15:00:16Z

Codecov Report

Attention: Patch coverage is 97.10145% with 2 lines in your changes missing coverage. Please review.

Project coverage is 68.86%. Comparing base (e8321ae) to head (7843794).
Report is 10 commits behind head on master.

Files with missing lines	Patch %	Lines
internal/network/utils.go	87.50%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #6978      +/-   ##
==========================================
+ Coverage   68.82%   68.86%   +0.04%     
==========================================
  Files         251      251              
  Lines       37630    37684      +54     
==========================================
+ Hits        25897    25952      +55     
+ Misses       9416     9412       -4     
- Partials     2317     2320       +3

Files with missing lines	Coverage Δ
internal/host/hostcommands/install_cmd.go	`86.28% <100.00%> (-0.39%)`	⬇️
internal/host/refresh_status_preprocessor.go	`94.39% <100.00%> (+0.07%)`	⬆️
internal/host/statemachine.go	`100.00% <100.00%> (ø)`
internal/host/validation_id.go	`90.90% <100.00%> (ø)`
internal/host/validator.go	`81.80% <100.00%> (+0.34%)`	⬆️
internal/network/machine_network_cidr.go	`60.59% <100.00%> (+4.92%)`	⬆️
internal/network/utils.go	`44.48% <87.50%> (+2.59%)`	⬆️

... and 9 files with indirect coverage changes

adriengentil · 2024-11-08T16:51:18Z

/cc @rccrdpccl @AlonaKaplan @eifrach

…n use In order to successfully install an OCP cluster using iSCSI boot disk, we need to ensure that the machine networks doesn't belong the IP addresses set on the network interface connected to the iSCSI disk. In a previous effort (openshift#6434), we ensured that the iSCSI disk can be selected as installation disk only if the machine has 2 network interfaces.

rccrdpccl · 2024-11-08T20:41:41Z

/retest

eifrach · 2024-11-11T07:57:00Z

 	HostValidationIDNoIPCollisionsInNetwork HostValidationID = "no-ip-collisions-in-network"
+
+	// HostValidationIDIscsiHostNetworkInterfaceDoesNotBelongToMachineCidr captures enum value "iscsi-host-network-interface-does-not-belong-to-machine-cidr"
+	HostValidationIDIscsiHostNetworkInterfaceDoesNotBelongToMachineCidr HostValidationID = "iscsi-host-network-interface-does-not-belong-to-machine-cidr"


I really think that a short name will help here

eifrach

Great Work, small changes just to make it easier to read / deubg

I also want you to add deubg output for the Expect I didn't comment on all of them.

eifrach · 2024-11-12T14:44:31Z

let's check if it's ClusterManagedNetworking retrun true
cause we allow only one network

why? It was not there before?

ignore it - It OK
you can have different host network form ISCSI network

…chine networks

openshift-ci · 2024-11-12T20:23:00Z

@adriengentil: all tests passed!

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-sigs/prow repository. I understand the commands that are listed here.

rccrdpccl · 2024-11-13T08:55:52Z

/lgtm

openshift-bot · 2024-11-13T11:26:31Z

[ART PR BUILD NOTIFIER]

Distgit: ose-agent-installer-api-server
This PR has been included in build ose-agent-installer-api-server-container-v4.18.0-202411131039.p0.g086bd74.assembly.stream.el9.
All builds following this will include this PR.

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 Nov 8, 2024

openshift-ci Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. api-review Categorizes an issue or PR as actively needing an API review. labels Nov 8, 2024

openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 8, 2024

adriengentil force-pushed the iscsi-machine-networks branch from 3b3391b to 0525314 Compare November 8, 2024 12:11

adriengentil changed the title ~~iscsi machine networks~~ MGMT-18513: Add machine networks validation when iSCSI boot disk is in use Nov 8, 2024

openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Nov 8, 2024

adriengentil force-pushed the iscsi-machine-networks branch from 0525314 to 2b4626f Compare November 8, 2024 13:55

adriengentil force-pushed the iscsi-machine-networks branch from 2b4626f to 1d0ee57 Compare November 8, 2024 16:50

openshift-ci Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 8, 2024

openshift-ci Bot requested review from AlonaKaplan, eifrach and rccrdpccl November 8, 2024 16:51

adriengentil marked this pull request as ready for review November 8, 2024 16:51

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 Nov 8, 2024

openshift-ci Bot requested a review from mlorenzofr November 8, 2024 16:51

adriengentil force-pushed the iscsi-machine-networks branch from 1d0ee57 to 25d8677 Compare November 8, 2024 16:53

rccrdpccl reviewed Nov 8, 2024

View reviewed changes

Comment thread internal/host/validator.go Outdated

rccrdpccl reviewed Nov 8, 2024

View reviewed changes

Comment thread internal/host/validations_test.go

rccrdpccl reviewed Nov 8, 2024

View reviewed changes

Comment thread internal/host/validator.go Outdated

$eifrach$

eifrach reviewed Nov 10, 2024

View reviewed changes

Comment thread swagger.yaml Outdated