openshift / assisted-service

Apache License 2.0
109 stars 209 forks source link

MGMT-18514: Calculate machine networks in external platform #6661

Closed adriengentil closed 1 month ago

adriengentil commented 1 month ago

Since external platform relies on user managed networking, we need the feature introduced in https://github.com/openshift/assisted-service/pull/6257 to determine the machine networks.

This change is useful when using iSCSI boot drive on Oracle Cloud Infrastructure, in order to select the machine network that doesn't belong to the iSCSI subnet.

openshift-ci-robot commented 1 month ago

@adriengentil: This pull request references MGMT-18514 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.17.0" version, but no target version was set.

In response to [this](https://github.com/openshift/assisted-service/pull/6661): >Since external platform relies on user managed networking, we need the >feature introduced in >https://github.com/openshift/assisted-service/pull/6257 to determine the >machine networks. > >This change is useful when using iSCSI boot drive on Oracle Cloud >Infrastructure, in order to select the machine network that doesn't >belonmg to the iSCSI subnet. > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fassisted-service). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
openshift-ci[bot] commented 1 month ago

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

adriengentil commented 1 month ago

/test edge-unit-test edge-lint edge-subsystem-aws

openshift-ci[bot] commented 1 month ago

[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

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/openshift/assisted-service/blob/master/OWNERS)~~ [adriengentil] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
adriengentil commented 1 month ago

/test edge-unit-test edge-lint

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 68.61%. Comparing base (d79f85c) to head (2069cf3). Report is 4 commits behind head on master.

Files Patch % Lines
internal/provider/external/base.go 0.00% 8 Missing :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/openshift/assisted-service/pull/6661/graphs/tree.svg?width=650&height=150&src=pr&token=YOR4NSSOXQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift)](https://app.codecov.io/gh/openshift/assisted-service/pull/6661?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift) ```diff @@ Coverage Diff @@ ## master #6661 +/- ## ========================================== + Coverage 68.56% 68.61% +0.05% ========================================== Files 246 246 Lines 36770 36796 +26 ========================================== + Hits 25211 25248 +37 + Misses 9317 9301 -16 - Partials 2242 2247 +5 ``` | [Files](https://app.codecov.io/gh/openshift/assisted-service/pull/6661?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift) | Coverage Δ | | |---|---|---| | [internal/provider/external/external.go](https://app.codecov.io/gh/openshift/assisted-service/pull/6661?src=pr&el=tree&filepath=internal%2Fprovider%2Fexternal%2Fexternal.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift#diff-aW50ZXJuYWwvcHJvdmlkZXIvZXh0ZXJuYWwvZXh0ZXJuYWwuZ28=) | `0.00% <ø> (ø)` | | | [internal/provider/external/oci.go](https://app.codecov.io/gh/openshift/assisted-service/pull/6661?src=pr&el=tree&filepath=internal%2Fprovider%2Fexternal%2Foci.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift#diff-aW50ZXJuYWwvcHJvdmlkZXIvZXh0ZXJuYWwvb2NpLmdv) | `84.61% <ø> (+32.23%)` | :arrow_up: | | [internal/provider/external/base.go](https://app.codecov.io/gh/openshift/assisted-service/pull/6661?src=pr&el=tree&filepath=internal%2Fprovider%2Fexternal%2Fbase.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift#diff-aW50ZXJuYWwvcHJvdmlkZXIvZXh0ZXJuYWwvYmFzZS5nbw==) | `0.00% <0.00%> (ø)` | | ... and [5 files with indirect coverage changes](https://app.codecov.io/gh/openshift/assisted-service/pull/6661/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=openshift)
adriengentil commented 1 month ago

/test edge-e2e-metal-assisted-external

adriengentil commented 1 month ago

/test edge-e2e-oci-assisted

openshift-ci-robot commented 1 month ago

@adriengentil: This pull request references MGMT-18514 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.

In response to [this](https://github.com/openshift/assisted-service/pull/6661): >Since external platform relies on user managed networking, we need the >feature introduced in >https://github.com/openshift/assisted-service/pull/6257 to determine the >machine networks. > >This change is useful when using iSCSI boot drive on Oracle Cloud >Infrastructure, in order to select the machine network that doesn't >belong to the iSCSI subnet. > Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Fassisted-service). If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
adriengentil commented 1 month ago

@ori-amizur would it make sense to call this new function ConfigureUserManagedNetworkingInInstallConfig from Nutanix and VSphere ?

adriengentil commented 1 month ago

/test edge-unit-test edge-lint edge-e2e-oci-assisted

ori-amizur commented 1 month ago

would it make sense to call this new function ConfigureUserManagedNetworkingInInstallConfig from Nutanix and VSphere ?

I am not familiar so much with Vsphere and Nutanix. If these platforms behave the same as none-platform then yes.

ori-amizur commented 1 month ago

/lgtm

adriengentil commented 1 month ago

/retest

openshift-bot commented 1 month ago

[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-202408122145.p0.g00a5eb3.assembly.stream.el9. All builds following this will include this PR.