search

openshift / installer

Install an OpenShift 4.x cluster
https://try.openshift.com
Apache License 2.0
1.44k stars 1.39k forks source link

CORS-2754: ipv4.internalJoinSubnet Support #9212

Closed patrickdillon closed 4 days ago

patrickdillon commented 1 week ago

Adds support for customizing the v4InternalSubnet for ovn-kubernetes with an install config field:

for example

networking:
  ovnKubernetesConfig:
    ipv4:
      internalJoinSubnet: 101.64.0.0/16

would produce

# cat c/manifests/cluster-network-03-config.yml 
# cat c/manifests/cluster-network-03-config.yml 
apiVersion: operator.openshift.io/v1
kind: Network
metadata:
  creationTimestamp: null
  name: cluster
spec:
  clusterNetwork:
  - cidr: 10.128.0.0/14
    hostPrefix: 23
  defaultNetwork:
    ovnKubernetesConfig:
      egressIPConfig: {}
      ipv4:
        internalJoinSubnet: 101.64.0.0/16
    type: OVNKubernetes
  disableNetworkDiagnostics: false
  managementState: Managed
  observedConfig: null
  serviceNetwork:
  - 172.30.0.0/16
  unsupportedConfigOverrides: null
status:
  readyReplicas: 0
openshift-ci-robot commented 1 week ago

@patrickdillon: This pull request references CORS-2754 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 story to target the "4.18.0" version, but no target version was set.

In response to [this](https://github.com/openshift/installer/pull/9212): >Adds support for customizing the v4InternalSubnet for ovn-kubernetes with an install config field: > >for example > >```yaml >networking: > ovnKubernetesConfig: > v4InternalSubnet: 101.64.0.0/16 >``` > >would produce >```shell ># cat c/manifests/cluster-network-03-config.yml >``` > >```yaml >apiVersion: operator.openshift.io/v1 >kind: Network >metadata: > creationTimestamp: null > name: cluster >spec: > clusterNetwork: > - cidr: 10.128.0.0/14 > hostPrefix: 23 > defaultNetwork: > ovnKubernetesConfig: > egressIPConfig: {} > v4InternalSubnet: 101.64.0.0/16 > type: OVNKubernetes > disableNetworkDiagnostics: false > managementState: Managed > observedConfig: null > serviceNetwork: > - 172.30.0.0/16 > unsupportedConfigOverrides: null >status: > readyReplicas: 0 >``` Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Finstaller). 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.
patrickdillon commented 1 week ago

/test ?

openshift-ci[bot] commented 1 week ago

@patrickdillon: The following commands are available to trigger required jobs:

The following commands are available to trigger optional jobs:

Use /test all to run the following jobs that were automatically triggered:

In response to [this](https://github.com/openshift/installer/pull/9212#issuecomment-2483326091): >/test ? Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
patrickdillon commented 1 week ago

PowerVS apparently always sets this manifest:

/test e2e-powervs-capi-ovn

Also, need to rename these tests

patrickdillon commented 1 week ago

/cc @mtulio Marco would you take a look here? I needed to rework some of the things that you added for custom MTU/edge nodes.

patrickdillon commented 1 week ago

/test e2e-powervs-capi-ovn

mtulio commented 1 week ago

/cc @mtulio Marco would you take a look here? I needed to rework some of the things that you added for custom MTU/edge nodes.

@patrickdillon , sure! Thanks for pinging. If you don't mind, let me trigger an specific e2e for the mtu while reviewing.

/test e2e-aws-overlay-mtu-ovn-1200

patrickdillon commented 1 week ago

ug will fix the fresh linting and codegen errors after tests run. they are minor.

patrickdillon commented 1 week ago

I have finished all the changes I intend except for the linting/codegen minor fix. So PTAL!

mtulio commented 6 days ago

/test e2e-aws-ovn-edge-zones

patrickdillon commented 5 days ago

/test e2e-powervs-capi-ovn

patrickdillon commented 5 days ago

/hold Need to make some updates to the api

openshift-ci-robot commented 5 days ago

@patrickdillon: This pull request references CORS-2754 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 story to target the "4.18.0" version, but no target version was set.

In response to [this](https://github.com/openshift/installer/pull/9212): >Adds support for customizing the v4InternalSubnet for ovn-kubernetes with an install config field: > >for example > >```yaml >networking: > ovnKubernetesConfig: > ipv4: > internalJoinSubnet: 101.64.0.0/16 >``` > >would produce >```shell ># cat c/manifests/cluster-network-03-config.yml >``` > >```yaml ># cat c/manifests/cluster-network-03-config.yml >apiVersion: operator.openshift.io/v1 >kind: Network >metadata: > creationTimestamp: null > name: cluster >spec: > clusterNetwork: > - cidr: 10.128.0.0/14 > hostPrefix: 23 > defaultNetwork: > ovnKubernetesConfig: > egressIPConfig: {} > ipv4: > internalJoinSubnet: 101.64.0.0/16 > type: OVNKubernetes > disableNetworkDiagnostics: false > managementState: Managed > observedConfig: null > serviceNetwork: > - 172.30.0.0/16 > unsupportedConfigOverrides: null >status: > readyReplicas: 0 >``` Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=openshift%2Finstaller). 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.
patrickdillon commented 5 days ago

/hold cancel

During some further testing, I discovered that V4InternalSubnet will be deprecated soon (see https://github.com/openshift/api/pull/1626), so I updated the install config API to mirror the direction we are going, rather than that which we will soon deprecate.

It was pretty much a simple 1-1 replacement of ovnKubernetesConfig.V4InternalSubnet with ovnKubernetesConfig.ipv4.InternalJoinSubnet

patrickdillon commented 5 days ago

Tested a successful install in AWS & confirmed the internal join subnet is as specified in the install config

# oc describe networks.operator.openshift.io cluster | yq .Spec
Cluster Network:
  Cidr: 10.128.0.0/14
  Host Prefix: 23
Default Network:
  Ovn Kubernetes Config:
    Egress IP Config:
    Gateway Config:
      ipv4:
      ipv6:
      Routing Via Host: false
    Geneve Port: 6081
    Ipsec Config:
      Mode: Disabled
    ipv4:
      Internal Join Subnet: 101.64.0.0/16
    Mtu: 8901
    Policy Audit Config:
      Destination: null
      Max File Size: 50
      Max Log Files: 5
      Rate Limit: 20
      Syslog Facility: local0
  Type: OVNKubernetes
Deploy Kube Proxy: false
Disable Multi Network: false
Disable Network Diagnostics: false
Log Level: Normal
Management State: Managed
Observed Config: <nil>
Operator Log Level: Normal
Service Network: 172.30.0.0/16
Unsupported Config Overrides: <nil>
Use Multi Network Policy: false
patrickdillon commented 5 days ago

/test e2e-powervs-capi-ovn

sadasu commented 5 days ago

/approve

A couple of minor comments which can be fixed when golint errors are fixed.

openshift-ci[bot] commented 5 days ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sadasu

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/installer/blob/master/OWNERS)~~ [sadasu] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
patrickdillon commented 4 days ago

/test e2e-powervs-capi-ovn

2uasimojo commented 4 days ago

@patrickdillon can you confirm that this feature is fully enabled through install-config.yaml? I.e. manifest patching is not necessary?

patrickdillon commented 4 days ago

@patrickdillon can you confirm that this feature is fully enabled through install-config.yaml? I.e. manifest patching is not necessary?

correct. everything is handled via install config:

networking:
  ovnKubernetesConfig:
    ipv4:
      internalJoinSubnet: 101.64.0.0/16
patrickdillon commented 4 days ago

/label acknowledge-critical-fixes-only

openshift-ci[bot] commented 4 days ago

@patrickdillon: The following test 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-powervs-capi-ovn dbee680bec66012453faec17c72decd70c7b336e link false /test e2e-powervs-capi-ovn

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
openshift-bot commented 4 days ago

[ART PR BUILD NOTIFIER]

Distgit: ose-installer-altinfra This PR has been included in build ose-installer-altinfra-container-v4.19.0-202411212037.p0.g545c830.assembly.stream.el9. All builds following this will include this PR.

openshift-bot commented 4 days ago

[ART PR BUILD NOTIFIER]

Distgit: ose-installer-terraform-providers This PR has been included in build ose-installer-terraform-providers-container-v4.19.0-202411212037.p0.g545c830.assembly.stream.el9. All builds following this will include this PR.

openshift-bot commented 4 days ago

[ART PR BUILD NOTIFIER]

Distgit: ose-baremetal-installer This PR has been included in build ose-baremetal-installer-container-v4.19.0-202411212037.p0.g545c830.assembly.stream.el9. All builds following this will include this PR.

openshift-bot commented 4 days ago

[ART PR BUILD NOTIFIER]

Distgit: ose-installer-artifacts This PR has been included in build ose-installer-artifacts-container-v4.19.0-202411212037.p0.g545c830.assembly.stream.el9. All builds following this will include this PR.