diff --git a/data/data/manifests/openshift/cluster-network-crd.yaml b/data/data/manifests/openshift/cluster-network-crd.yaml new file mode 100644 index 0000000000..ad4d8f932f --- /dev/null +++ b/data/data/manifests/openshift/cluster-network-crd.yaml @@ -0,0 +1,16 @@ +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: networks.config.openshift.io +spec: + group: config.openshift.io + names: + kind: Network + listKind: NetworkList + plural: networks + singular: network + scope: Cluster + versions: + - name: v1 + served: true + storage: true diff --git a/data/data/manifests/openshift/cluster-networkconfig-crd.yaml b/data/data/manifests/openshift/cluster-networkconfig-crd.yaml new file mode 100644 index 0000000000..e35c7cf56f --- /dev/null +++ b/data/data/manifests/openshift/cluster-networkconfig-crd.yaml @@ -0,0 +1,19 @@ +# This is the advanced network configuration CRD +# Only necessary if you need to tweak certain settings. +# See https://github.com/openshift/cluster-network-operator#configuring +apiVersion: apiextensions.k8s.io/v1beta1 +kind: CustomResourceDefinition +metadata: + name: networkconfigs.networkoperator.openshift.io +spec: + group: networkoperator.openshift.io + names: + kind: NetworkConfig + listKind: NetworkConfigList + plural: networkconfigs + singular: networkconfig + scope: Cluster + versions: + - name: v1 + served: true + storage: true diff --git a/docs/user/troubleshooting.md b/docs/user/troubleshooting.md index 27cbdd1159..ad6bbc9106 100644 --- a/docs/user/troubleshooting.md +++ b/docs/user/troubleshooting.md @@ -224,19 +224,18 @@ From a deployment perspective, the network operator is often the "canary in the First, determine that the network configuration exists: ```console -$ kubectl get networkconfigs.networkoperator.openshift.io default -oyaml -... +$ kubectl get network.config.openshift.io cluster -oyaml +apiVersion: config.openshift.io/v1 +kind: Network +metadata: + name: cluster spec: - additionalNetworks: null - clusterNetworks: - - cidr: 10.2.0.0/16 - hostSubnetLength: 9 - defaultNetwork: - openshiftSDNConfig: - mode: Networkpolicy - otherConfig: null - type: OpenshiftSDN - serviceNetwork: 10.3.0.0/16 + serviceNetwork: + - 172.30.0.0/16 + clusterNetwork: + - cidr: 10.128.0.0/14 + hostPrefix: 23 + networkType: OpenShiftSDN ``` If it doesn't exist, the installer didn't create it. You'll have to run `openshift-install create manifests` to determine why. diff --git a/pkg/asset/installconfig/installconfig_test.go b/pkg/asset/installconfig/installconfig_test.go index 1874e82403..c61d4317d9 100644 --- a/pkg/asset/installconfig/installconfig_test.go +++ b/pkg/asset/installconfig/installconfig_test.go @@ -6,7 +6,6 @@ import ( "testing" "github.com/golang/mock/gomock" - netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -66,11 +65,11 @@ func TestInstallConfigGenerate_FillsInDefaults(t *testing.T) { BaseDomain: "test-domain", Networking: &types.Networking{ MachineCIDR: ipnet.MustParseCIDR("10.0.0.0/16"), - Type: "OpenshiftSDN", + Type: "OpenShiftSDN", ServiceCIDR: ipnet.MustParseCIDR("172.30.0.0/16"), - ClusterNetworks: []netopv1.ClusterNetwork{ + ClusterNetworks: []types.ClusterNetworkEntry{ { - CIDR: "10.128.0.0/14", + CIDR: *ipnet.MustParseCIDR("10.128.0.0/14"), HostSubnetLength: 9, }, }, @@ -125,11 +124,11 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}" BaseDomain: "test-domain", Networking: &types.Networking{ MachineCIDR: ipnet.MustParseCIDR("10.0.0.0/16"), - Type: "OpenshiftSDN", + Type: "OpenShiftSDN", ServiceCIDR: ipnet.MustParseCIDR("172.30.0.0/16"), - ClusterNetworks: []netopv1.ClusterNetwork{ + ClusterNetworks: []types.ClusterNetworkEntry{ { - CIDR: "10.128.0.0/14", + CIDR: *ipnet.MustParseCIDR("10.128.0.0/14"), HostSubnetLength: 9, }, }, diff --git a/pkg/asset/manifests/network.go b/pkg/asset/manifests/network.go index cfd4f21ba2..8fd338c887 100644 --- a/pkg/asset/manifests/network.go +++ b/pkg/asset/manifests/network.go @@ -1,16 +1,18 @@ package manifests import ( + "fmt" "os" "path/filepath" "github.com/ghodss/yaml" "github.com/pkg/errors" + configv1 "github.com/openshift/api/config/v1" "github.com/openshift/installer/pkg/asset" "github.com/openshift/installer/pkg/asset/installconfig" + "github.com/openshift/installer/pkg/asset/templates/content/openshift" - netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" clusterv1a1 "sigs.k8s.io/cluster-api/pkg/apis/cluster/v1alpha1" ) @@ -20,35 +22,18 @@ var ( noCfgFilename = filepath.Join(manifestDir, "cluster-network-02-config.yml") ) -const ( - - // We need to manually create our CRD first, so we can create the - // configuration instance of it. - // Other operators have their CRD created by the CVO, but we manually - // create our operator's configuration in the installer. - netConfigCRD = ` -apiVersion: apiextensions.k8s.io/v1beta1 -kind: CustomResourceDefinition -metadata: - name: networkconfigs.networkoperator.openshift.io -spec: - group: networkoperator.openshift.io - names: - kind: NetworkConfig - listKind: NetworkConfigList - plural: networkconfigs - singular: networkconfig - scope: Cluster - versions: - - name: v1 - served: true - storage: true -` -) +// We need to manually create our CRDs first, so we can create the +// configuration instance of it in the installer. Other operators have +// their CRD created by the CVO, but we need to create the corresponding +// CRs in the installer, so we need the CRD to be there. +// The first CRD is the high-level Network.config.openshift.io object, +// which is stable ahd minimal. Administrators can override configure the +// network in a more detailed manner with the operator-specific CR, which +// also needs to be done before the installer is run, so we provide both. // Networking generates the cluster-network-*.yml files. type Networking struct { - config *netopv1.NetworkConfig + config *configv1.Network FileList []*asset.File } @@ -64,60 +49,44 @@ func (no *Networking) Name() string { func (no *Networking) Dependencies() []asset.Asset { return []asset.Asset{ &installconfig.InstallConfig{}, + &openshift.NetworkCRDs{}, } } // Generate generates the network operator config and its CRD. func (no *Networking) Generate(dependencies asset.Parents) error { installConfig := &installconfig.InstallConfig{} - dependencies.Get(installConfig) + crds := &openshift.NetworkCRDs{} + dependencies.Get(installConfig, crds) netConfig := installConfig.Config.Networking - // determine pod address space. - // This can go away when we get rid of PodCIDR - // entirely in favor of ClusterNetworks - var clusterNets []netopv1.ClusterNetwork + clusterNet := []configv1.ClusterNetworkEntry{} if len(netConfig.ClusterNetworks) > 0 { - clusterNets = netConfig.ClusterNetworks - } else if !netConfig.PodCIDR.IPNet.IP.IsUnspecified() { - clusterNets = []netopv1.ClusterNetwork{ - { - CIDR: netConfig.PodCIDR.String(), - HostSubnetLength: 9, - }, + for _, net := range netConfig.ClusterNetworks { + _, size := net.CIDR.Mask.Size() + clusterNet = append(clusterNet, configv1.ClusterNetworkEntry{ + CIDR: net.CIDR.String(), + HostPrefix: uint32(size) - uint32(net.HostSubnetLength), + }) } } else { - return errors.Errorf("Either PodCIDR or ClusterNetworks must be specified") + return errors.Errorf("ClusterNetworks must be specified") } - defaultNet := netopv1.DefaultNetworkDefinition{ - Type: netConfig.Type, - } - - // Add any network-specific configuration defaults here. - switch netConfig.Type { - case netopv1.NetworkTypeOpenshiftSDN: - defaultNet.OpenshiftSDNConfig = &netopv1.OpenshiftSDNConfig{ - // Default to network policy, operator provides all other defaults. - Mode: netopv1.SDNModePolicy, - } - } - - no.config = &netopv1.NetworkConfig{ + no.config = &configv1.Network{ TypeMeta: metav1.TypeMeta{ - APIVersion: netopv1.SchemeGroupVersion.String(), - Kind: "NetworkConfig", + APIVersion: configv1.SchemeGroupVersion.String(), + Kind: "Network", }, ObjectMeta: metav1.ObjectMeta{ - Name: "default", + Name: "cluster", // not namespaced }, - - Spec: netopv1.NetworkConfigSpec{ - ServiceNetwork: netConfig.ServiceCIDR.String(), - ClusterNetworks: clusterNets, - DefaultNetwork: defaultNet, + Spec: configv1.NetworkSpec{ + ClusterNetwork: clusterNet, + ServiceNetwork: []string{netConfig.ServiceCIDR.String()}, + NetworkType: netConfig.Type, }, } @@ -126,10 +95,15 @@ func (no *Networking) Generate(dependencies asset.Parents) error { return errors.Wrapf(err, "failed to create %s manifests from InstallConfig", no.Name()) } + crdContents := "" + for _, crdFile := range crds.Files() { + crdContents = fmt.Sprintf("%s\n---\n%s", crdContents, crdFile.Data) + } + no.FileList = []*asset.File{ { Filename: noCrdFilename, - Data: []byte(netConfigCRD), + Data: []byte(crdContents), }, { Filename: noCfgFilename, @@ -155,13 +129,13 @@ func (no *Networking) ClusterNetwork() (*clusterv1a1.ClusterNetworkingConfig, er } pods := []string{} - for _, cn := range no.config.Spec.ClusterNetworks { + for _, cn := range no.config.Spec.ClusterNetwork { pods = append(pods, cn.CIDR) } cn := &clusterv1a1.ClusterNetworkingConfig{ Services: clusterv1a1.NetworkRanges{ - CIDRBlocks: []string{no.config.Spec.ServiceNetwork}, + CIDRBlocks: no.config.Spec.ServiceNetwork, }, Pods: clusterv1a1.NetworkRanges{ CIDRBlocks: pods, @@ -189,7 +163,7 @@ func (no *Networking) Load(f asset.FileFetcher) (bool, error) { return false, err } - netConfig := &netopv1.NetworkConfig{} + netConfig := &configv1.Network{} if err := yaml.Unmarshal(cfgFile.Data, netConfig); err != nil { return false, errors.Wrapf(err, "failed to unmarshal %s", noCfgFilename) } diff --git a/pkg/asset/templates/content/openshift/cluster-network-crds.go b/pkg/asset/templates/content/openshift/cluster-network-crds.go new file mode 100644 index 0000000000..f19efcb28c --- /dev/null +++ b/pkg/asset/templates/content/openshift/cluster-network-crds.go @@ -0,0 +1,69 @@ +package openshift + +import ( + "os" + "path/filepath" + + "github.com/openshift/installer/pkg/asset" + "github.com/openshift/installer/pkg/asset/templates/content" +) + +const ( + netCRDfilename = "cluster-network-crd.yaml" + netopCRDfilename = "cluster-networkconfig-crd.yaml" +) + +var _ asset.WritableAsset = (*NetworkCRDs)(nil) + +// NetworkCRDs is the custom resource definitions for the network operator types: +// - Network.config.openshift.io +// - NetworkConfig.networkoperator.openshift.io +type NetworkCRDs struct { + FileList []*asset.File +} + +// Dependencies returns all of the dependencies directly needed by the asset +func (t *NetworkCRDs) Dependencies() []asset.Asset { + return []asset.Asset{} +} + +// Name returns the human-friendly name of the asset. +func (t *NetworkCRDs) Name() string { + return "Network CRDs" +} + +// Generate generates the actual files by this asset +func (t *NetworkCRDs) Generate(parents asset.Parents) error { + for _, filename := range []string{netCRDfilename, netopCRDfilename} { + data, err := content.GetOpenshiftTemplate(filename) + if err != nil { + return err + } + t.FileList = append(t.FileList, &asset.File{ + Filename: filepath.Join(content.TemplateDir, filename), + Data: []byte(data), + }) + } + return nil +} + +// Files returns the files generated by the asset. +func (t *NetworkCRDs) Files() []*asset.File { + return t.FileList +} + +// Load returns the asset from disk. +func (t *NetworkCRDs) Load(f asset.FileFetcher) (bool, error) { + for _, filename := range []string{netCRDfilename, netopCRDfilename} { + file, err := f.FetchByName(filepath.Join(content.TemplateDir, filename)) + if err != nil { + if os.IsNotExist(err) { + return false, nil + } + return false, err + } + t.FileList = append(t.FileList, file) + } + + return true, nil +} diff --git a/pkg/asset/templates/templates.go b/pkg/asset/templates/templates.go index 87f160e5d2..82f9eaccd9 100644 --- a/pkg/asset/templates/templates.go +++ b/pkg/asset/templates/templates.go @@ -42,6 +42,7 @@ func (m *Templates) Dependencies() []asset.Asset { &openshift.KubeadminPasswordSecret{}, &openshift.RoleCloudCredsSecretReader{}, &openshift.InfrastructureCRD{}, + &openshift.NetworkCRDs{}, } } diff --git a/pkg/types/defaults/installconfig.go b/pkg/types/defaults/installconfig.go index e36864903b..62862b6265 100644 --- a/pkg/types/defaults/installconfig.go +++ b/pkg/types/defaults/installconfig.go @@ -1,8 +1,6 @@ package defaults import ( - netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" - "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types" awsdefaults "github.com/openshift/installer/pkg/types/aws/defaults" @@ -15,8 +13,9 @@ import ( var ( defaultMachineCIDR = ipnet.MustParseCIDR("10.0.0.0/16") defaultServiceCIDR = ipnet.MustParseCIDR("172.30.0.0/16") - defaultClusterCIDR = "10.128.0.0/14" + defaultClusterCIDR = ipnet.MustParseCIDR("10.128.0.0/14") defaultHostSubnetLength = 9 // equivalent to a /23 per node + defaultNetworkPlugin = "OpenShiftSDN" ) // SetInstallConfigDefaults sets the defaults for the install config. @@ -31,16 +30,16 @@ func SetInstallConfigDefaults(c *types.InstallConfig) { } } if c.Networking.Type == "" { - c.Networking.Type = netopv1.NetworkTypeOpenshiftSDN + c.Networking.Type = defaultNetworkPlugin } if c.Networking.ServiceCIDR == nil { c.Networking.ServiceCIDR = defaultServiceCIDR } - if len(c.Networking.ClusterNetworks) == 0 && c.Networking.PodCIDR == nil { - c.Networking.ClusterNetworks = []netopv1.ClusterNetwork{ + if len(c.Networking.ClusterNetworks) == 0 { + c.Networking.ClusterNetworks = []types.ClusterNetworkEntry{ { - CIDR: defaultClusterCIDR, - HostSubnetLength: uint32(defaultHostSubnetLength), + CIDR: *defaultClusterCIDR, + HostSubnetLength: int32(defaultHostSubnetLength), }, } } diff --git a/pkg/types/defaults/installconfig_test.go b/pkg/types/defaults/installconfig_test.go index bfb1caeeb0..b42abedfb1 100644 --- a/pkg/types/defaults/installconfig_test.go +++ b/pkg/types/defaults/installconfig_test.go @@ -3,7 +3,6 @@ package defaults import ( "testing" - netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" "github.com/stretchr/testify/assert" "github.com/openshift/installer/pkg/ipnet" @@ -23,12 +22,12 @@ func defaultInstallConfig() *types.InstallConfig { return &types.InstallConfig{ Networking: &types.Networking{ MachineCIDR: defaultMachineCIDR, - Type: netopv1.NetworkTypeOpenshiftSDN, + Type: defaultNetworkPlugin, ServiceCIDR: defaultServiceCIDR, - ClusterNetworks: []netopv1.ClusterNetwork{ + ClusterNetworks: []types.ClusterNetworkEntry{ { - CIDR: defaultClusterCIDR, - HostSubnetLength: uint32(defaultHostSubnetLength), + CIDR: *defaultClusterCIDR, + HostSubnetLength: int32(defaultHostSubnetLength), }, }, }, @@ -127,12 +126,12 @@ func TestSetInstallConfigDefaults(t *testing.T) { name: "Networking types present", config: &types.InstallConfig{ Networking: &types.Networking{ - Type: netopv1.NetworkType("test-networking-type"), + Type: "test-networking-type", }, }, expected: func() *types.InstallConfig { c := defaultInstallConfig() - c.Networking.Type = netopv1.NetworkType("test-networking-type") + c.Networking.Type = "test-networking-type" return c }(), }, @@ -153,9 +152,9 @@ func TestSetInstallConfigDefaults(t *testing.T) { name: "Cluster Networks present", config: &types.InstallConfig{ Networking: &types.Networking{ - ClusterNetworks: []netopv1.ClusterNetwork{ + ClusterNetworks: []types.ClusterNetworkEntry{ { - CIDR: "test-cidr", + CIDR: *ipnet.MustParseCIDR("8.8.0.0/18"), HostSubnetLength: 10, }, }, @@ -163,29 +162,15 @@ func TestSetInstallConfigDefaults(t *testing.T) { }, expected: func() *types.InstallConfig { c := defaultInstallConfig() - c.Networking.ClusterNetworks = []netopv1.ClusterNetwork{ + c.Networking.ClusterNetworks = []types.ClusterNetworkEntry{ { - CIDR: "test-cidr", + CIDR: *ipnet.MustParseCIDR("8.8.0.0/18"), HostSubnetLength: 10, }, } return c }(), }, - { - name: "Pod CIDR present", - config: &types.InstallConfig{ - Networking: &types.Networking{ - PodCIDR: ipnet.MustParseCIDR("1.2.3.4/8"), - }, - }, - expected: func() *types.InstallConfig { - c := defaultInstallConfig() - c.Networking.ClusterNetworks = nil - c.Networking.PodCIDR = ipnet.MustParseCIDR("1.2.3.4/8") - return c - }(), - }, { name: "Machines present", config: &types.InstallConfig{ diff --git a/pkg/types/installconfig.go b/pkg/types/installconfig.go index 25fab10c71..54b4f9c309 100644 --- a/pkg/types/installconfig.go +++ b/pkg/types/installconfig.go @@ -1,7 +1,6 @@ package types import ( - netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" "github.com/openshift/installer/pkg/ipnet" "github.com/openshift/installer/pkg/types/aws" "github.com/openshift/installer/pkg/types/libvirt" @@ -120,8 +119,8 @@ type Networking struct { // Type is the network type to install // +optional - // Default is OpenshiftSDN. - Type netopv1.NetworkType `json:"type,omitempty"` + // Default is OpenShiftSDN. + Type string `json:"type,omitempty"` // ServiceCIDR is the IP address space from which to assign service IPs. // +optional @@ -133,12 +132,16 @@ type Networking struct { // Default is a single cluster network with a CIDR of 10.128.0.0/14 // and a host subnet length of 9. The default is only applicable if PodCIDR // is not present. - ClusterNetworks []netopv1.ClusterNetwork `json:"clusterNetworks,omitempty"` - - // PodCIDR is deprecated (and badly named; it should have always - // been called ClusterCIDR. If no ClusterNetworks are specified, - // we will fall back to the PodCIDR - // TODO(cdc) remove this. - // +optional - PodCIDR *ipnet.IPNet `json:"podCIDR,omitempty"` + ClusterNetworks []ClusterNetworkEntry `json:"clusterNetworks,omitempty"` +} + +// ClusterNetworkEntry is a single IP address block for pod IP blocks. IP blocks +// are allocated with size 2^HostSubnetLength. +type ClusterNetworkEntry struct { + // The IP block address pool + CIDR ipnet.IPNet `json:"cidr"` + + // The size of blocks to allocate from the larger pool. + // This is the length in bits - so a 9 here will allocate a /23. + HostSubnetLength int32 `json:"hostSubnetLength"` } diff --git a/pkg/types/validation/installconfig.go b/pkg/types/validation/installconfig.go index 17528c5f40..33de9af47b 100644 --- a/pkg/types/validation/installconfig.go +++ b/pkg/types/validation/installconfig.go @@ -6,7 +6,6 @@ import ( "sort" "strings" - netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" "k8s.io/apimachinery/pkg/util/validation/field" "github.com/openshift/installer/pkg/types" @@ -58,8 +57,8 @@ func ValidateInstallConfig(c *types.InstallConfig, openStackValidValuesFetcher o func validateNetworking(n *types.Networking, fldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if !validate.ValidNetworkTypes[n.Type] { - allErrs = append(allErrs, field.NotSupported(fldPath.Child("type"), n.Type, validate.ValidNetworkTypeValues)) + if n.Type == "" { + allErrs = append(allErrs, field.Required(fldPath.Child("type"), "network provider type required")) } if err := validate.SubnetCIDR(&n.MachineCIDR.IPNet); err != nil { allErrs = append(allErrs, field.Invalid(fldPath.Child("machineCIDR"), n.MachineCIDR, err.Error())) @@ -70,35 +69,22 @@ func validateNetworking(n *types.Networking, fldPath *field.Path) field.ErrorLis for i, cn := range n.ClusterNetworks { allErrs = append(allErrs, validateClusterNetwork(&cn, fldPath.Child("clusterNetworks").Index(i), &n.ServiceCIDR.IPNet)...) } - if n.PodCIDR != nil { - if err := validate.SubnetCIDR(&n.PodCIDR.IPNet); err != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("podCIDR"), n.PodCIDR, err.Error())) - } - if validate.DoCIDRsOverlap(&n.ServiceCIDR.IPNet, &n.PodCIDR.IPNet) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("podCIDR"), n.PodCIDR, "podCIDR must not overlap with serviceCIDR")) - } - } - if len(n.ClusterNetworks) == 0 && n.PodCIDR == nil { - allErrs = append(allErrs, field.Invalid(fldPath, n, "either clusterNetworks or podCIDR is required")) - } - if len(n.ClusterNetworks) != 0 && n.PodCIDR != nil { - allErrs = append(allErrs, field.Invalid(fldPath.Child("podCIDR"), n.PodCIDR, "cannot use podCIDR when clusterNetworks is used")) + if len(n.ClusterNetworks) == 0 { + allErrs = append(allErrs, field.Required(fldPath.Child("clusterNetworks"), "cluster network required")) } return allErrs } -func validateClusterNetwork(cn *netopv1.ClusterNetwork, fldPath *field.Path, serviceCIDR *net.IPNet) field.ErrorList { +func validateClusterNetwork(cn *types.ClusterNetworkEntry, fldPath *field.Path, serviceCIDR *net.IPNet) field.ErrorList { allErrs := field.ErrorList{} - _, cidr, err := net.ParseCIDR(cn.CIDR) - if err == nil { - if validate.DoCIDRsOverlap(cidr, serviceCIDR) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("cidr"), cn.CIDR, "cluster network CIDR must not overlap with serviceCIDR")) - } - if ones, bits := cidr.Mask.Size(); cn.HostSubnetLength > uint32(bits-ones) { - allErrs = append(allErrs, field.Invalid(fldPath.Child("hostSubnetLength"), cn.HostSubnetLength, "cluster network host subnet length must not be greater than CIDR length")) - } - } else { - allErrs = append(allErrs, field.Invalid(fldPath.Child("cidr"), cn.CIDR, err.Error())) + if validate.DoCIDRsOverlap(&cn.CIDR.IPNet, serviceCIDR) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("cidr"), cn.CIDR.String(), "cluster network CIDR must not overlap with serviceCIDR")) + } + if cn.HostSubnetLength < 0 { + allErrs = append(allErrs, field.Invalid(fldPath.Child("hostSubnetLength"), cn.HostSubnetLength, "hostSubnetLength must be positive")) + } + if ones, bits := cn.CIDR.Mask.Size(); cn.HostSubnetLength > int32(bits-ones) { + allErrs = append(allErrs, field.Invalid(fldPath.Child("hostSubnetLength"), cn.HostSubnetLength, "cluster network host subnet must not be larger than CIDR "+cn.CIDR.String())) } return allErrs } diff --git a/pkg/types/validation/installconfig_test.go b/pkg/types/validation/installconfig_test.go index 776cf5797b..f23d742558 100644 --- a/pkg/types/validation/installconfig_test.go +++ b/pkg/types/validation/installconfig_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/golang/mock/gomock" - netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" "github.com/stretchr/testify/assert" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -26,12 +25,12 @@ func validInstallConfig() *types.InstallConfig { }, BaseDomain: "test-domain", Networking: &types.Networking{ - Type: "OpenshiftSDN", + Type: "OpenShiftSDN", MachineCIDR: ipnet.MustParseCIDR("10.0.0.0/16"), ServiceCIDR: ipnet.MustParseCIDR("172.30.0.0/16"), - ClusterNetworks: []netopv1.ClusterNetwork{ + ClusterNetworks: []types.ClusterNetworkEntry{ { - CIDR: "192.168.1.0/24", + CIDR: *ipnet.MustParseCIDR("192.168.1.0/24"), HostSubnetLength: 4, }, }, @@ -117,10 +116,10 @@ func TestValidateInstallConfig(t *testing.T) { name: "invalid network type", installConfig: func() *types.InstallConfig { c := validInstallConfig() - c.Networking.Type = "bad-type" + c.Networking.Type = "" return c }(), - expectedError: `^networking.type: Unsupported value: "bad-type": supported values: "Calico", "Kuryr", "OVNKubernetes", "OpenshiftSDN"$`, + expectedError: `^networking.type: Required value: network provider type required$`, }, { name: "invalid service cidr", @@ -131,33 +130,24 @@ func TestValidateInstallConfig(t *testing.T) { }(), expectedError: `^networking\.serviceCIDR: Invalid value: ipnet\.IPNet{IPNet:net\.IPNet{IP:net\.IP\(nil\), Mask:net\.IPMask\(nil\)}}: must use IPv4$`, }, - { - name: "invalid cluster network cidr", - installConfig: func() *types.InstallConfig { - c := validInstallConfig() - c.Networking.ClusterNetworks[0].CIDR = "bad-cidr" - return c - }(), - expectedError: `^networking\.clusterNetworks\[0]\.cidr: Invalid value: "bad-cidr": invalid CIDR address: bad-cidr$`, - }, { name: "overlapping cluster network cidr", installConfig: func() *types.InstallConfig { c := validInstallConfig() - c.Networking.ClusterNetworks[0].CIDR = "172.30.0.0/24" + c.Networking.ClusterNetworks[0].CIDR = *ipnet.MustParseCIDR("172.30.2.0/24") return c }(), - expectedError: `^networking\.clusterNetworks\[0]\.cidr: Invalid value: "172\.30\.0\.0/24": cluster network CIDR must not overlap with serviceCIDR$`, + expectedError: `^networking\.clusterNetworks\[0]\.cidr: Invalid value: "172\.30\.2\.0/24": cluster network CIDR must not overlap with serviceCIDR$`, }, { name: "cluster network host subnet length too large", installConfig: func() *types.InstallConfig { c := validInstallConfig() - c.Networking.ClusterNetworks[0].CIDR = "192.168.1.0/24" + c.Networking.ClusterNetworks[0].CIDR = *ipnet.MustParseCIDR("192.168.1.0/24") c.Networking.ClusterNetworks[0].HostSubnetLength = 9 return c }(), - expectedError: `^networking\.clusterNetworks\[0]\.hostSubnetLength: Invalid value: 0x9: cluster network host subnet length must not be greater than CIDR length$`, + expectedError: `^networking\.clusterNetworks\[0]\.hostSubnetLength: Invalid value: 9: cluster network host subnet must not be larger than CIDR 192.168.1.0/24$`, }, { name: "missing master machine pool", diff --git a/pkg/validate/validate.go b/pkg/validate/validate.go index 0602596d8c..940299c3f1 100644 --- a/pkg/validate/validate.go +++ b/pkg/validate/validate.go @@ -7,10 +7,8 @@ import ( "fmt" "net" "net/url" - "sort" "strings" - netopv1 "github.com/openshift/cluster-network-operator/pkg/apis/networkoperator/v1" "golang.org/x/crypto/ssh" k8serrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation" @@ -21,27 +19,6 @@ var ( _, cidr, _ := net.ParseCIDR("172.17.0.0/16") return cidr }() - - // ValidNetworkTypes is a collection of the valid network types. - ValidNetworkTypes = map[netopv1.NetworkType]bool{ - netopv1.NetworkTypeOpenshiftSDN: true, - netopv1.NetworkTypeOVNKubernetes: true, - netopv1.NetworkTypeCalico: true, - netopv1.NetworkTypeKuryr: true, - } - - // ValidNetworkTypeValues is a slice filled with the valid network types as - // strings. - ValidNetworkTypeValues = func() []string { - validValues := make([]string, len(ValidNetworkTypes)) - i := 0 - for t := range ValidNetworkTypes { - validValues[i] = string(t) - i++ - } - sort.Strings(validValues) - return validValues - }() ) func validateSubdomain(v string) error {