Skip to content

Commit

Permalink
[release-4.14] OCPBUGS-23069: Ignore hostPrefix validation for plugin…
Browse files Browse the repository at this point in the history
…s other than OVN/SDN (#5676)

* OCPBUGS-19823: Ignore hostPrefix validation for plugins other than OVN/SDN For plugins other than OVN/SDN (e.g. Calico), the clusterNetwork hostPrefix setting is not used so remove the validation. Since the networkType for plugins other than OVN/SDN can be passed in installconfig-overrides, that must also be checked.

Note that this is similar to the installer change to ignore the hostPrefix for
these networkTypes - openshift/installer#3888.

* updated per review comments

* change to only skip validation based on networkType

* ensure that cluster cidr size verification is still done

* move order of of checks for NetworkType

---------

Co-authored-by: Bob Fournier <bfournie@redhat.com>
  • Loading branch information
openshift-cherrypick-robot and bfournie committed Dec 1, 2023
1 parent 4e1a1e5 commit 0634e0a
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 2 deletions.
43 changes: 41 additions & 2 deletions internal/cluster/validator.go
@@ -1,6 +1,7 @@
package cluster

import (
"encoding/json"
"fmt"
"reflect"
"strings"
Expand All @@ -11,6 +12,7 @@ import (
"github.com/hashicorp/go-multierror"
"github.com/openshift/assisted-service/internal/common"
"github.com/openshift/assisted-service/internal/host"
"github.com/openshift/assisted-service/internal/installcfg"
"github.com/openshift/assisted-service/internal/network"
"github.com/openshift/assisted-service/internal/usage"
"github.com/openshift/assisted-service/models"
Expand Down Expand Up @@ -533,10 +535,19 @@ func (v *clusterValidator) networkPrefixValid(c *clusterPreprocessContext) (Vali
if !validationStatusToBool(clusterCidrDefined) {
return ValidationPending, "The Cluster Network CIDR is undefined."
}

skipHostPrefix := v.skipNetworkHostPrefixCheck(c)

validClusterNetworks := funk.Filter(c.cluster.ClusterNetworks, func(clusterNetwork *models.ClusterNetwork) bool {
var hostPrefixForCidrValidation int
if !skipHostPrefix {
hostPrefixForCidrValidation = int(clusterNetwork.HostPrefix)
} else {
hostPrefixForCidrValidation = 25 // default to 128 addresses
}
return clusterNetwork != nil &&
network.VerifyNetworkHostPrefix(clusterNetwork.HostPrefix) == nil &&
network.VerifyClusterCidrSize(int(clusterNetwork.HostPrefix), string(clusterNetwork.Cidr), len(c.cluster.Hosts)) == nil
(skipHostPrefix || network.VerifyNetworkHostPrefix(clusterNetwork.HostPrefix) == nil) &&
network.VerifyClusterCidrSize(hostPrefixForCidrValidation, string(clusterNetwork.Cidr), len(c.cluster.Hosts)) == nil
}).([]*models.ClusterNetwork)

if len(validClusterNetworks) == len(c.cluster.ClusterNetworks) {
Expand Down Expand Up @@ -566,3 +577,31 @@ func (v *clusterValidator) isNtpServerConfigured(c *clusterPreprocessContext) (V
return ValidationFailure, fmt.Sprintf("Hosts' clocks are not synchronized (there's more than a %d minutes gap between clocks), "+
"please configure an NTP server via DHCP or set clocks manually.", common.MaximumAllowedTimeDiffMinutes)
}

// Ignore hostPrefix for non-OVN/SDN plugins.
func (v *clusterValidator) skipNetworkHostPrefixCheck(c *clusterPreprocessContext) bool {
// list of known plugins that require hostPrefix to be set
var pluginsUsingHostPrefix = []string{models.ClusterNetworkTypeOVNKubernetes, models.ClusterNetworkTypeOpenShiftSDN}

networkType := swag.StringValue(c.cluster.NetworkType)
if c.cluster.InstallConfigOverrides != "" {
// use networkType from install-config overrides if set
overrideDecoder := json.NewDecoder(strings.NewReader(c.cluster.InstallConfigOverrides))
overrideDecoder.DisallowUnknownFields()

cfg := &installcfg.InstallerConfigBaremetal{}
if overrideDecoder.Decode(cfg) != nil {
v.log.Infof("could not decode install-config overrides %s", c.cluster.InstallConfigOverrides)
return false
}

if cfg.Networking.NetworkType != "" {
networkType = cfg.Networking.NetworkType
}
}
if !funk.ContainsString(pluginsUsingHostPrefix, networkType) {
v.log.Infof("skipping network prefix check for %s", networkType)
return true
}
return false
}
51 changes: 51 additions & 0 deletions internal/cluster/validator_test.go
Expand Up @@ -612,3 +612,54 @@ var _ = Describe("Platform validations", func() {
Expect(message).To(Equal("Platform requirements satisfied"))
})
})

var _ = Describe("skipNetworkHostPrefixCheck", func() {

var (
validator clusterValidator
preprocessContext *clusterPreprocessContext
clusterID strfmt.UUID
)

BeforeEach(func() {
validator = clusterValidator{logrus.New(), nil}
preprocessContext = &clusterPreprocessContext{}
clusterID = strfmt.UUID(uuid.New().String())
})

It("Returns false when hostPrefix 0 and networkType OVN", func() {
clusterNetworks := []*models.ClusterNetwork{
{Cidr: "10.0.2.1/24", ClusterID: clusterID},
{Cidr: "2002::1234:abcd:ffff:c0a8:102/64", ClusterID: clusterID},
}
networkType := models.ClusterNetworkTypeOVNKubernetes

preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{
ID: &clusterID,
ClusterNetworks: clusterNetworks,
NetworkType: &networkType,
}}

skipped := validator.skipNetworkHostPrefixCheck(preprocessContext)
Expect(skipped).Should(Equal(false))
})

It("Returns true when hostPrefix 0 and networkType not OVN or SDN", func() {
clusterNetworks := []*models.ClusterNetwork{
{Cidr: "10.0.2.1/24", ClusterID: clusterID},
{Cidr: "2002::1234:abcd:ffff:c0a8:102/64", ClusterID: clusterID},
}
networkType := models.ClusterNetworkTypeOVNKubernetes
installCfgOverrides := "{\"networking\":{\"networkType\":\"Calico\"}}"

preprocessContext.cluster = &common.Cluster{Cluster: models.Cluster{
ID: &clusterID,
ClusterNetworks: clusterNetworks,
NetworkType: &networkType,
InstallConfigOverrides: installCfgOverrides,
}}

skipped := validator.skipNetworkHostPrefixCheck(preprocessContext)
Expect(skipped).Should(Equal(true))
})
})

0 comments on commit 0634e0a

Please sign in to comment.