From b93cda478d63b154ae2aef1f4dc3eda4f3b946c3 Mon Sep 17 00:00:00 2001 From: a-palchikov Date: Mon, 13 Jul 2020 20:32:13 +0200 Subject: [PATCH] [6.1.x] always create ClusterConfiguration resource on install (#1864) * Add guard blocks to avoid use of nil interface * Always generate cluster configuration based on CLI flags even if only from default values * Include pod/service CIDR values in text output of 'gravity resource get' * Move podCIDR/serviceCIDR separate values to cluster configuration on configuration early on. This will remove the need to handle these attributes separately * Run resource validation early also on cluster configuration resource during installation * Address review comments --- e | 2 +- lib/install/config.go | 10 +- lib/install/engine/cli/cli.go | 6 +- lib/ops/agents.go | 2 +- lib/ops/agents_test.go | 2 +- lib/ops/opsservice/configure.go | 20 ++- lib/ops/resources/gravity/collection.go | 20 ++- lib/ops/resources/gravity/gravity.go | 8 +- lib/storage/clusterconfig/clusterconfig.go | 7 + lib/validate/net.go | 10 ++ tool/gravity/cli/config.go | 150 +++++++++++++-------- tool/gravity/cli/install.go | 4 +- 12 files changed, 155 insertions(+), 86 deletions(-) diff --git a/e b/e index 35b84c9855..59453248d2 160000 --- a/e +++ b/e @@ -1 +1 @@ -Subproject commit 35b84c98552d7e84eda7ba2f6ae94011202806c1 +Subproject commit 59453248d265decea6a93cdad0375b8e631329c9 diff --git a/lib/install/config.go b/lib/install/config.go index 1f65e29173..25e3266858 100644 --- a/lib/install/config.go +++ b/lib/install/config.go @@ -86,11 +86,11 @@ type Config struct { // SiteDomain is the name of the cluster SiteDomain string // Flavor is installation flavor - Flavor *schema.Flavor + Flavor schema.Flavor // Role is server role Role string // App is the application being installed - App *app.Application + App app.Application // RuntimeResources specifies optional Kubernetes resources to create RuntimeResources []runtime.Object // ClusterResources specifies optional cluster resources to create @@ -103,10 +103,6 @@ type Config struct { Mounts map[string]string // DNSOverrides contains installer node DNS overrides DNSOverrides storage.DNSOverrides - // PodCIDR is a pod network CIDR - PodCIDR string - // ServiceCIDR is a service network CIDR - ServiceCIDR string // VxlanPort is the overlay network port VxlanPort int // DNSConfig overrides the local cluster DNS configuration @@ -169,7 +165,7 @@ func (c *Config) checkAndSetDefaults() (err error) { if c.LocalBackend == nil { return trace.BadParameter("missing LocalBackend") } - if c.App == nil { + if c.App.Package.IsEmpty() { return trace.BadParameter("missing App") } if c.DNSConfig.IsEmpty() { diff --git a/lib/install/engine/cli/cli.go b/lib/install/engine/cli/cli.go index 31ac07c1cb..5e394e413b 100644 --- a/lib/install/engine/cli/cli.go +++ b/lib/install/engine/cli/cli.go @@ -171,12 +171,10 @@ func (r *executor) createOperation() (*ops.SiteOperation, error) { Docker: r.config.Docker, }, OnPrem: storage.OnPremVariables{ - PodCIDR: r.config.PodCIDR, - ServiceCIDR: r.config.ServiceCIDR, - VxlanPort: r.config.VxlanPort, + VxlanPort: r.config.VxlanPort, }, }, - Profiles: install.ServerRequirements(*r.config.Flavor), + Profiles: install.ServerRequirements(r.config.Flavor), }) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/ops/agents.go b/lib/ops/agents.go index ac87aa8bab..93037ca843 100644 --- a/lib/ops/agents.go +++ b/lib/ops/agents.go @@ -114,7 +114,7 @@ func (s *AgentReport) Diff(previous *AgentReport) (added, removed []checks.Serve // // Returns number/roles of agents that still need to join as well as any // extra servers that are not a part of the flavor. -func (s *AgentReport) MatchFlavor(flavor *schema.Flavor) (needed map[string]int, extra []checks.ServerInfo) { +func (s *AgentReport) MatchFlavor(flavor schema.Flavor) (needed map[string]int, extra []checks.ServerInfo) { needed = make(map[string]int) for _, node := range flavor.Nodes { needed[node.Profile] = node.Count diff --git a/lib/ops/agents_test.go b/lib/ops/agents_test.go index e5a7efee02..92306a146e 100644 --- a/lib/ops/agents_test.go +++ b/lib/ops/agents_test.go @@ -63,7 +63,7 @@ func (s *AgentReportSuite) TestCheck(c *check.C) { Servers: []checks.ServerInfo{server1, server2, server3}, } - needed, extra := report.MatchFlavor(&schema.Flavor{ + needed, extra := report.MatchFlavor(schema.Flavor{ Nodes: []schema.FlavorNode{ {Profile: "worker", Count: 3}, {Profile: "db", Count: 2}, diff --git a/lib/ops/opsservice/configure.go b/lib/ops/opsservice/configure.go index 74571dee58..a187f39347 100644 --- a/lib/ops/opsservice/configure.go +++ b/lib/ops/opsservice/configure.go @@ -317,15 +317,15 @@ func (s *site) configurePackages(ctx *operationContext, req ops.ConfigurePackage etcdConfig := s.prepareEtcdConfig(ctx) - var clusterConfig clusterconfig.Interface + clusterConfig := clusterconfig.NewEmpty() if len(req.Config) != 0 { clusterConfig, err = clusterconfig.Unmarshal(req.Config) if err != nil { return trace.Wrap(err) } - if s.cloudProviderName() != "" { - clusterConfig.SetCloudProvider(s.cloudProviderName()) - } + } + if s.cloudProviderName() != "" { + clusterConfig.SetCloudProvider(s.cloudProviderName()) } for i, master := range masters { @@ -1037,11 +1037,19 @@ func (s *site) configurePlanetServer(config planetConfig) error { } func (r planetConfig) podSubnet() string { - return podSubnet(r.installExpand.InstallExpand, r.config.GetGlobalConfig().PodCIDR) + var override string + if r.config != nil { + override = r.config.GetGlobalConfig().PodCIDR + } + return podSubnet(r.installExpand.InstallExpand, override) } func (r planetConfig) serviceSubnet() string { - return serviceSubnet(r.installExpand.InstallExpand, r.config.GetGlobalConfig().ServiceCIDR) + var override string + if r.config != nil { + override = r.config.GetGlobalConfig().ServiceCIDR + } + return serviceSubnet(r.installExpand.InstallExpand, override) } type planetConfig struct { diff --git a/lib/ops/resources/gravity/collection.go b/lib/ops/resources/gravity/collection.go index f6374c8fa6..01de1c6e68 100644 --- a/lib/ops/resources/gravity/collection.go +++ b/lib/ops/resources/gravity/collection.go @@ -664,13 +664,11 @@ func (r configCollection) WriteText(w io.Writer) error { fmt.Fprintf(t, "%v\n", string(config.Config)) } config := r.GetGlobalConfig() - displayCloudConfig := config.CloudProvider != "" || config.CloudConfig != "" - if displayCloudConfig { - common.PrintCustomTableHeader(t, []string{"Cloud"}, "-") - if len(config.CloudProvider) != 0 { - fmt.Fprintf(t, "Provider:\t%v\n", config.CloudProvider) - } - formatCloudConfig(t, config.CloudConfig) + if len(config.PodCIDR) != 0 { + fmt.Fprintf(t, "Pod IP Range:\t%v\n", config.PodCIDR) + } + if len(config.ServiceCIDR) != 0 { + fmt.Fprintf(t, "Service IP Range:\t%v\n", config.ServiceCIDR) } if len(config.ServiceNodePortRange) != 0 { fmt.Fprintf(t, "Service Node Port Range:\t%v\n", config.ServiceNodePortRange) @@ -681,6 +679,14 @@ func (r configCollection) WriteText(w io.Writer) error { if len(config.FeatureGates) != 0 { fmt.Fprintf(t, "Feature Gates:\t%v\n", formatFeatureGates(config.FeatureGates)) } + displayCloudConfig := config.CloudProvider != "" || config.CloudConfig != "" + if displayCloudConfig { + common.PrintCustomTableHeader(t, []string{"Cloud"}, "-") + if len(config.CloudProvider) != 0 { + fmt.Fprintf(t, "Provider:\t%v\n", config.CloudProvider) + } + formatCloudConfig(t, config.CloudConfig) + } _, err := io.WriteString(w, t.String()) return trace.Wrap(err) } diff --git a/lib/ops/resources/gravity/gravity.go b/lib/ops/resources/gravity/gravity.go index e28cae42d3..d7712733ad 100644 --- a/lib/ops/resources/gravity/gravity.go +++ b/lib/ops/resources/gravity/gravity.go @@ -25,6 +25,7 @@ import ( "github.com/gravitational/gravity/lib/ops/resources" "github.com/gravitational/gravity/lib/storage" "github.com/gravitational/gravity/lib/storage/clusterconfig" + "github.com/gravitational/gravity/lib/validate" "github.com/fatih/color" teleservices "github.com/gravitational/teleport/lib/services" @@ -559,7 +560,12 @@ func Validate(resource storage.UnknownResource) (err error) { case storage.KindRuntimeEnvironment: _, err = storage.UnmarshalEnvironmentVariables(resource.Raw) case storage.KindClusterConfiguration: - _, err = clusterconfig.Unmarshal(resource.Raw) + config, err := clusterconfig.Unmarshal(resource.Raw) + if err != nil { + return trace.Wrap(err) + } + globalConfig := config.GetGlobalConfig() + return validate.KubernetesSubnetsFromStrings(globalConfig.PodCIDR, globalConfig.ServiceCIDR) default: return trace.NotImplemented("unsupported resource %q, supported are: %v", resource.Kind, modules.GetResources().SupportedResources()) diff --git a/lib/storage/clusterconfig/clusterconfig.go b/lib/storage/clusterconfig/clusterconfig.go index 8332d6463b..04c1265177 100644 --- a/lib/storage/clusterconfig/clusterconfig.go +++ b/lib/storage/clusterconfig/clusterconfig.go @@ -41,6 +41,8 @@ type Interface interface { GetKubeletConfig() *Kubelet // GetGlobalConfig returns the global configuration GetGlobalConfig() Global + // SetGlobalConfig sets the new global configuration + SetGlobalConfig(Global) // SetCloudProvider sets the cloud provider for this configuration SetCloudProvider(provider string) } @@ -111,6 +113,11 @@ func (r *Resource) GetGlobalConfig() Global { return r.Spec.Global } +// SetGlobalConfig sets the new global configuration +func (r *Resource) SetGlobalConfig(config Global) { + r.Spec.Global = config +} + // SetCloudProvider sets the cloud provider for this configuration func (r *Resource) SetCloudProvider(provider string) { r.Spec.Global.CloudProvider = provider diff --git a/lib/validate/net.go b/lib/validate/net.go index 1f75d65f3b..022e8a5871 100644 --- a/lib/validate/net.go +++ b/lib/validate/net.go @@ -72,3 +72,13 @@ func KubernetesSubnets(podNet, serviceNet *net.IPNet) (err error) { } return nil } + +// NormalizeSubnet returns the text representation of the given subnet +// as the IP masked with the network mask +func NormalizeSubnet(subnet string) (string, error) { + _, ipNet, err := net.ParseCIDR(subnet) + if err != nil { + return "", trace.Wrap(err) + } + return ipNet.String(), nil +} diff --git a/tool/gravity/cli/config.go b/tool/gravity/cli/config.go index 1052f3a149..3135b5e9b3 100644 --- a/tool/gravity/cli/config.go +++ b/tool/gravity/cli/config.go @@ -61,6 +61,7 @@ import ( "github.com/gravitational/gravity/lib/systemservice" "github.com/gravitational/gravity/lib/users" "github.com/gravitational/gravity/lib/utils" + "github.com/gravitational/gravity/lib/validate" gcemeta "cloud.google.com/go/compute/metadata" "github.com/docker/docker/pkg/namesgenerator" @@ -157,9 +158,22 @@ type InstallConfig struct { FromService bool // AcceptEULA allows to auto-accept end-user license agreement. AcceptEULA bool + + // Computed values + // // writeStateDir is the directory where installer stores state for the duration // of the operation writeStateDir string + // app is the application being installed + app app.Application + // kubernetesResources lists additional Kubernetes resources supplied on command line + kubernetesResources []runtime.Object + // gravityResources lists additional Gravity resources supplied on command line + gravityResources []storage.UnknownResource + // dnsOverrides lists additional cluster DNS host/zone overrides + dnsOverrides storage.DNSOverrides + // flavor specifies the selected application flavor + flavor schema.Flavor } // NewInstallConfig creates install config from the passed CLI args and flags @@ -211,7 +225,7 @@ func NewInstallConfig(env *localenv.LocalEnvironment, g *Application) InstallCon } // CheckAndSetDefaults validates the configuration object and populates default values -func (i *InstallConfig) CheckAndSetDefaults() (err error) { +func (i *InstallConfig) CheckAndSetDefaults(validator resources.Validator) (err error) { if i.FieldLogger == nil { i.FieldLogger = log.WithField(trace.Component, "installer") } @@ -294,10 +308,47 @@ func (i *InstallConfig) CheckAndSetDefaults() (err error) { if i.DNSConfig.IsEmpty() { i.DNSConfig = storage.DefaultDNSConfig } + app, err := i.getApp() + if err != nil { + return trace.Wrap(err) + } + i.app = *app err = i.checkEULA() if err != nil { return trace.Wrap(err) } + err = validate.KubernetesSubnetsFromStrings(i.PodCIDR, i.ServiceCIDR) + if err != nil { + return trace.Wrap(err) + } + flavor, err := getFlavor(i.Flavor, i.app.Manifest, i.FieldLogger) + if err != nil { + return trace.Wrap(err) + } + i.flavor = *flavor + i.Role, err = validateRole(i.Role, i.flavor, i.app.Manifest.NodeProfiles, i.FieldLogger) + if err != nil { + return trace.Wrap(err) + } + if !i.Remote { + // Compute the cloud provider before updating the cluster configuration + // so it reflects the autodetected value as well + if err := i.validateCloudConfig(i.app.Manifest); err != nil { + return trace.Wrap(err) + } + } + dnsOverrides, err := i.getDNSOverrides() + if err != nil { + return trace.Wrap(err) + } + i.dnsOverrides = *dnsOverrides + if i.SiteDomain == "" { + i.SiteDomain = generateClusterName() + } + err = i.validateResources(validator) + if err != nil { + return trace.Wrap(err) + } return nil } @@ -307,11 +358,7 @@ func (i *InstallConfig) checkEULA() error { if i.Mode != constants.InstallModeCLI || i.FromService { return nil } - app, err := i.getApp() - if err != nil { - return trace.Wrap(err) - } - eula := app.Manifest.EULA() + eula := i.app.Manifest.EULA() if eula == "" { return nil } @@ -355,42 +402,7 @@ func (i *InstallConfig) NewInstallerConfig( env *localenv.LocalEnvironment, wizard *localenv.RemoteEnvironment, process process.GravityProcess, - validator resources.Validator, ) (*install.Config, error) { - var kubernetesResources []runtime.Object - var gravityResources []storage.UnknownResource - if i.ResourcesPath != "" { - var err error - kubernetesResources, gravityResources, err = i.splitResources(validator) - if err != nil { - return nil, trace.Wrap(err) - } - } - app, err := i.getApp() - if err != nil { - return nil, trace.Wrap(err) - } - dnsOverrides, err := i.getDNSOverrides() - if err != nil { - return nil, trace.Wrap(err) - } - gravityResources, err = i.updateClusterConfig(gravityResources) - if err != nil { - return nil, trace.Wrap(err) - } - flavor, err := getFlavor(i.Flavor, app.Manifest, i.FieldLogger) - if err != nil { - return nil, trace.Wrap(err) - } - i.Role, err = validateRole(i.Role, *flavor, app.Manifest.NodeProfiles, i.FieldLogger) - if err != nil { - return nil, trace.Wrap(err) - } - if !i.Remote { - if err := i.validateCloudConfig(app.Manifest); err != nil { - return nil, trace.Wrap(err) - } - } token, err := generateInstallToken(wizard.Operator, i.Token) if err != nil && !trace.IsAlreadyExists(err) { return nil, trace.Wrap(err) @@ -398,9 +410,6 @@ func (i *InstallConfig) NewInstallerConfig( if err := upsertSystemAccount(wizard.Operator); err != nil { return nil, trace.Wrap(err) } - if i.SiteDomain == "" { - i.SiteDomain = generateClusterName() - } return &install.Config{ FieldLogger: i.FieldLogger, AdvertiseAddr: i.AdvertiseAddr, @@ -417,8 +426,6 @@ func (i *InstallConfig) NewInstallerConfig( SystemDevice: i.SystemDevice, Mounts: i.Mounts, DNSConfig: i.DNSConfig, - PodCIDR: i.PodCIDR, - ServiceCIDR: i.ServiceCIDR, VxlanPort: i.VxlanPort, Docker: i.Docker, Insecure: i.Insecure, @@ -426,11 +433,11 @@ func (i *InstallConfig) NewInstallerConfig( Role: i.Role, ServiceUser: *i.ServiceUser, Token: *token, - App: app, - Flavor: flavor, - DNSOverrides: *dnsOverrides, - RuntimeResources: kubernetesResources, - ClusterResources: gravityResources, + App: i.app, + Flavor: i.flavor, + DNSOverrides: i.dnsOverrides, + RuntimeResources: i.kubernetesResources, + ClusterResources: i.gravityResources, Process: process, Apps: wizard.Apps, Packages: wizard.Packages, @@ -554,6 +561,22 @@ func (i *InstallConfig) getDNSOverrides() (*storage.DNSOverrides, error) { return overrides, nil } +func (i *InstallConfig) validateResources(validator resources.Validator) (err error) { + var gravityResources []storage.UnknownResource + if i.ResourcesPath != "" { + var err error + i.kubernetesResources, gravityResources, err = i.splitResources(validator) + if err != nil { + return trace.Wrap(err) + } + } + i.gravityResources, err = i.updateClusterConfig(gravityResources) + if err != nil { + return trace.Wrap(err) + } + return nil +} + // splitResources validates the resources specified in ResourcePath // using the given validator and splits them into Kubernetes and Gravity-specific func (i *InstallConfig) splitResources(validator resources.Validator) (runtimeResources []runtime.Object, clusterResources []storage.UnknownResource, err error) { @@ -589,14 +612,14 @@ func (i *InstallConfig) updateClusterConfig(resources []storage.UnknownResource) } updated = append(updated, res) } - if clusterConfig == nil && i.CloudProvider == "" { - // Return the resources unchanged - return resources, nil - } var config clusterconfig.Interface if clusterConfig == nil { config = clusterconfig.New(clusterconfig.Spec{ - Global: clusterconfig.Global{CloudProvider: i.CloudProvider}, + Global: clusterconfig.Global{ + CloudProvider: i.CloudProvider, + ServiceCIDR: i.ServiceCIDR, + PodCIDR: i.PodCIDR, + }, }) } else { config, err = clusterconfig.Unmarshal(clusterConfig.Raw) @@ -608,6 +631,21 @@ func (i *InstallConfig) updateClusterConfig(resources []storage.UnknownResource) if globalConfig.CloudProvider != "" { i.CloudProvider = globalConfig.CloudProvider } + if i.ServiceCIDR != "" && globalConfig.ServiceCIDR == "" { + globalConfig.ServiceCIDR = i.ServiceCIDR + } + if i.PodCIDR != "" && globalConfig.PodCIDR == "" { + globalConfig.PodCIDR = i.PodCIDR + } + globalConfig.PodCIDR, err = validate.NormalizeSubnet(globalConfig.PodCIDR) + if err != nil { + return nil, trace.Wrap(err, "invalid pod subnet: %v", globalConfig.PodCIDR) + } + globalConfig.ServiceCIDR, err = validate.NormalizeSubnet(globalConfig.ServiceCIDR) + if err != nil { + return nil, trace.Wrap(err, "invalid service subnet: %v", globalConfig.ServiceCIDR) + } + config.SetGlobalConfig(globalConfig) // Serialize the cluster configuration and add to resources configResource, err := clusterconfig.ToUnknown(config) if err != nil { diff --git a/tool/gravity/cli/install.go b/tool/gravity/cli/install.go index 8088c8b2ac..bf6928beff 100644 --- a/tool/gravity/cli/install.go +++ b/tool/gravity/cli/install.go @@ -58,7 +58,7 @@ import ( func startInstall(env *localenv.LocalEnvironment, config InstallConfig) error { env.PrintStep("Starting installer") - if err := config.CheckAndSetDefaults(); err != nil { + if err := config.CheckAndSetDefaults(resources.ValidateFunc(gravity.Validate)); err != nil { return trace.Wrap(err) } if config.FromService { @@ -141,7 +141,7 @@ func newInstallerConfig(ctx context.Context, env *localenv.LocalEnvironment, con if err != nil { return nil, trace.Wrap(err) } - installerConfig, err := config.NewInstallerConfig(env, wizard, process, resources.ValidateFunc(gravity.Validate)) + installerConfig, err := config.NewInstallerConfig(env, wizard, process) if err != nil { return nil, trace.Wrap(err) }