From bacc06d8fccb0b15a1177d0ac44ba21a80509d3d Mon Sep 17 00:00:00 2001 From: Arvinderpal Wander Date: Wed, 4 Sep 2019 18:06:49 -0700 Subject: [PATCH] kubeadm --service-cluster-ip-range supports a comma seperated list of service subnets. Update DNS, Cert, dry-run logic to support list of Service CIDRs. Added unit tests for GetKubernetesServiceCIDR and updated GetDNSIP() unit test to inclue dual-sack cases. --- .../app/apis/kubeadm/validation/validation.go | 6 +- cmd/kubeadm/app/cmd/init.go | 6 +- cmd/kubeadm/app/componentconfigs/defaults.go | 2 +- cmd/kubeadm/app/constants/BUILD | 1 + cmd/kubeadm/app/constants/constants.go | 46 +++++++++++-- cmd/kubeadm/app/constants/constants_test.go | 67 +++++++++++++++++++ cmd/kubeadm/app/phases/addons/dns/BUILD | 1 + cmd/kubeadm/app/phases/addons/dns/dns.go | 5 +- cmd/kubeadm/app/phases/addons/dns/dns_test.go | 17 ++++- cmd/kubeadm/app/preflight/checks.go | 8 ++- cmd/kubeadm/app/util/pkiutil/BUILD | 2 +- cmd/kubeadm/app/util/pkiutil/pki_helpers.go | 12 +--- 12 files changed, 149 insertions(+), 24 deletions(-) diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation.go index bd595d69a7e..b37665a59c6 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go +++ b/cmd/kubeadm/app/apis/kubeadm/validation/validation.go @@ -414,8 +414,10 @@ func ValidateNetworking(c *kubeadm.ClusterConfiguration, fldPath *field.Path) fi } // check if dual-stack feature-gate is enabled isDualStack := features.Enabled(c.FeatureGates, features.IPv6DualStack) - // TODO(Arvinderpal): use isDualStack flag once list of service CIDRs is supported (PR: #79386) - allErrs = append(allErrs, ValidateIPNetFromString(c.Networking.ServiceSubnet, constants.MinimumAddressesInServiceSubnet, false /*isDualStack*/, field.NewPath("serviceSubnet"))...) + + if len(c.Networking.ServiceSubnet) != 0 { + allErrs = append(allErrs, ValidateIPNetFromString(c.Networking.ServiceSubnet, constants.MinimumAddressesInServiceSubnet, isDualStack, field.NewPath("serviceSubnet"))...) + } if len(c.Networking.PodSubnet) != 0 { allErrs = append(allErrs, ValidateIPNetFromString(c.Networking.PodSubnet, constants.MinimumAddressesInServiceSubnet, isDualStack, field.NewPath("podSubnet"))...) } diff --git a/cmd/kubeadm/app/cmd/init.go b/cmd/kubeadm/app/cmd/init.go index 03d2c508d18..622729ef8fe 100644 --- a/cmd/kubeadm/app/cmd/init.go +++ b/cmd/kubeadm/app/cmd/init.go @@ -512,8 +512,12 @@ func (d *initData) OutputWriter() io.Writer { func (d *initData) Client() (clientset.Interface, error) { if d.client == nil { if d.dryRun { + svcSubnetCIDR, err := kubeadmconstants.GetKubernetesServiceCIDR(d.cfg.Networking.ServiceSubnet, features.Enabled(d.cfg.FeatureGates, features.IPv6DualStack)) + if err != nil { + return nil, errors.Wrapf(err, "unable to get internal Kubernetes Service IP from the given service CIDR (%s)", d.cfg.Networking.ServiceSubnet) + } // If we're dry-running, we should create a faked client that answers some GETs in order to be able to do the full init flow and just logs the rest of requests - dryRunGetter := apiclient.NewInitDryRunGetter(d.cfg.NodeRegistration.Name, d.cfg.Networking.ServiceSubnet) + dryRunGetter := apiclient.NewInitDryRunGetter(d.cfg.NodeRegistration.Name, svcSubnetCIDR.String()) d.client = apiclient.NewDryRunClient(dryRunGetter, os.Stdout) } else { // If we're acting for real, we should create a connection to the API server and wait for it to come up diff --git a/cmd/kubeadm/app/componentconfigs/defaults.go b/cmd/kubeadm/app/componentconfigs/defaults.go index 516911bfb99..b0435fff3cc 100644 --- a/cmd/kubeadm/app/componentconfigs/defaults.go +++ b/cmd/kubeadm/app/componentconfigs/defaults.go @@ -113,7 +113,7 @@ func DefaultKubeletConfiguration(internalcfg *kubeadmapi.ClusterConfiguration) { } clusterDNS := "" - dnsIP, err := constants.GetDNSIP(internalcfg.Networking.ServiceSubnet) + dnsIP, err := constants.GetDNSIP(internalcfg.Networking.ServiceSubnet, features.Enabled(internalcfg.FeatureGates, features.IPv6DualStack)) if err != nil { clusterDNS = kubeadmapiv1beta2.DefaultClusterDNSIP } else { diff --git a/cmd/kubeadm/app/constants/BUILD b/cmd/kubeadm/app/constants/BUILD index acd3daff0d8..c064cfeb513 100644 --- a/cmd/kubeadm/app/constants/BUILD +++ b/cmd/kubeadm/app/constants/BUILD @@ -21,6 +21,7 @@ go_library( "//staging/src/k8s.io/apimachinery/pkg/util/version:go_default_library", "//staging/src/k8s.io/cluster-bootstrap/token/api:go_default_library", "//vendor/github.com/pkg/errors:go_default_library", + "//vendor/k8s.io/utils/net:go_default_library", ], ) diff --git a/cmd/kubeadm/app/constants/constants.go b/cmd/kubeadm/app/constants/constants.go index 83f3bb6389e..7f6e4623a81 100644 --- a/cmd/kubeadm/app/constants/constants.go +++ b/cmd/kubeadm/app/constants/constants.go @@ -23,6 +23,7 @@ import ( "os" "path" "path/filepath" + "strings" "time" "github.com/pkg/errors" @@ -31,6 +32,7 @@ import ( bootstrapapi "k8s.io/cluster-bootstrap/token/api" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" "k8s.io/kubernetes/pkg/registry/core/service/ipallocator" + utilnet "k8s.io/utils/net" ) const ( @@ -522,22 +524,58 @@ func CreateTimestampDirForKubeadm(kubernetesDir, dirName string) (string, error) } // GetDNSIP returns a dnsIP, which is 10th IP in svcSubnet CIDR range -func GetDNSIP(svcSubnet string) (net.IP, error) { +func GetDNSIP(svcSubnetList string, isDualStack bool) (net.IP, error) { // Get the service subnet CIDR - _, svcSubnetCIDR, err := net.ParseCIDR(svcSubnet) + svcSubnetCIDR, err := GetKubernetesServiceCIDR(svcSubnetList, isDualStack) if err != nil { - return nil, errors.Wrapf(err, "couldn't parse service subnet CIDR %q", svcSubnet) + return nil, errors.Wrapf(err, "unable to get internal Kubernetes Service IP from the given service CIDR (%s)", svcSubnetList) } // Selects the 10th IP in service subnet CIDR range as dnsIP dnsIP, err := ipallocator.GetIndexedIP(svcSubnetCIDR, 10) if err != nil { - return nil, errors.Wrapf(err, "unable to get tenth IP address from service subnet CIDR %s", svcSubnetCIDR.String()) + return nil, errors.Wrap(err, "unable to get internal Kubernetes Service IP from the given service CIDR") } return dnsIP, nil } +// GetKubernetesServiceCIDR returns the default Service CIDR for the Kubernetes internal service +func GetKubernetesServiceCIDR(svcSubnetList string, isDualStack bool) (*net.IPNet, error) { + if isDualStack { + // The default service address family for the cluster is the address family of the first + // service cluster IP range configured via the `--service-cluster-ip-range` flag + // of the kube-controller-manager and kube-apiserver. + svcSubnets, err := utilnet.ParseCIDRs(strings.Split(svcSubnetList, ",")) + if err != nil { + return nil, errors.Wrapf(err, "unable to parse ServiceSubnet %v", svcSubnetList) + } + if len(svcSubnets) == 0 { + return nil, errors.New("received empty ServiceSubnet for dual-stack") + } + return svcSubnets[0], nil + } + // internal IP address for the API server + _, svcSubnet, err := net.ParseCIDR(svcSubnetList) + if err != nil { + return nil, errors.Wrapf(err, "unable to parse ServiceSubnet %v", svcSubnetList) + } + return svcSubnet, nil +} + +// GetAPIServerVirtualIP returns the IP of the internal Kubernetes API service +func GetAPIServerVirtualIP(svcSubnetList string, isDualStack bool) (net.IP, error) { + svcSubnet, err := GetKubernetesServiceCIDR(svcSubnetList, isDualStack) + if err != nil { + return nil, errors.Wrap(err, "unable to get internal Kubernetes Service IP from the given service CIDR") + } + internalAPIServerVirtualIP, err := ipallocator.GetIndexedIP(svcSubnet, 1) + if err != nil { + return nil, errors.Wrapf(err, "unable to get the first IP address from the given CIDR: %s", svcSubnet.String()) + } + return internalAPIServerVirtualIP, nil +} + // GetStaticPodAuditPolicyFile returns the path to the audit policy file within a static pod func GetStaticPodAuditPolicyFile() string { return filepath.Join(KubernetesDir, AuditPolicyDir, AuditPolicyFile) diff --git a/cmd/kubeadm/app/constants/constants_test.go b/cmd/kubeadm/app/constants/constants_test.go index f8590962221..607a1a83974 100644 --- a/cmd/kubeadm/app/constants/constants_test.go +++ b/cmd/kubeadm/app/constants/constants_test.go @@ -225,3 +225,70 @@ func TestGetKubeDNSVersion(t *testing.T) { }) } } + +func TestGetKubernetesServiceCIDR(t *testing.T) { + var tests = []struct { + svcSubnetList string + isDualStack bool + expected string + expectedError bool + name string + }{ + { + svcSubnetList: "192.168.10.0/24", + isDualStack: false, + expected: "192.168.10.0/24", + expectedError: false, + name: "valid: valid IPv4 range from single-stack", + }, + { + svcSubnetList: "fd03::/112", + isDualStack: false, + expected: "fd03::/112", + expectedError: false, + name: "valid: valid IPv6 range from single-stack", + }, + { + svcSubnetList: "192.168.10.0/24,fd03::/112", + isDualStack: true, + expected: "192.168.10.0/24", + expectedError: false, + name: "valid: valid ranges from dual-stack", + }, + { + svcSubnetList: "fd03::/112,192.168.10.0/24", + isDualStack: true, + expected: "fd03::/112", + expectedError: false, + name: "valid: valid ranges from dual-stack", + }, + { + svcSubnetList: "192.168.10.0/24,fd03:x::/112", + isDualStack: true, + expected: "", + expectedError: true, + name: "invalid: failed to parse subnet range for dual-stack", + }, + } + + for _, rt := range tests { + t.Run(rt.name, func(t *testing.T) { + actual, actualError := GetKubernetesServiceCIDR(rt.svcSubnetList, rt.isDualStack) + if rt.expectedError { + if actualError == nil { + t.Errorf("failed GetKubernetesServiceCIDR:\n\texpected error, but got no error") + } + } else if !rt.expectedError && actualError != nil { + t.Errorf("failed GetKubernetesServiceCIDR:\n\texpected no error, but got: %v", actualError) + } else { + if actual.String() != rt.expected { + t.Errorf( + "failed GetKubernetesServiceCIDR:\n\texpected: %s\n\t actual: %s", + rt.expected, + actual.String(), + ) + } + } + }) + } +} diff --git a/cmd/kubeadm/app/phases/addons/dns/BUILD b/cmd/kubeadm/app/phases/addons/dns/BUILD index ab416a0de9e..80e132ce3a7 100644 --- a/cmd/kubeadm/app/phases/addons/dns/BUILD +++ b/cmd/kubeadm/app/phases/addons/dns/BUILD @@ -35,6 +35,7 @@ go_library( deps = [ "//cmd/kubeadm/app/apis/kubeadm:go_default_library", "//cmd/kubeadm/app/constants:go_default_library", + "//cmd/kubeadm/app/features:go_default_library", "//cmd/kubeadm/app/images:go_default_library", "//cmd/kubeadm/app/util:go_default_library", "//cmd/kubeadm/app/util/apiclient:go_default_library", diff --git a/cmd/kubeadm/app/phases/addons/dns/dns.go b/cmd/kubeadm/app/phases/addons/dns/dns.go index d37654ddc1c..39e8cf9e401 100644 --- a/cmd/kubeadm/app/phases/addons/dns/dns.go +++ b/cmd/kubeadm/app/phases/addons/dns/dns.go @@ -38,6 +38,7 @@ import ( "k8s.io/klog" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" + "k8s.io/kubernetes/cmd/kubeadm/app/features" "k8s.io/kubernetes/cmd/kubeadm/app/images" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" "k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" @@ -92,7 +93,7 @@ func kubeDNSAddon(cfg *kubeadmapi.ClusterConfiguration, client clientset.Interfa return err } - dnsip, err := kubeadmconstants.GetDNSIP(cfg.Networking.ServiceSubnet) + dnsip, err := kubeadmconstants.GetDNSIP(cfg.Networking.ServiceSubnet, features.Enabled(cfg.FeatureGates, features.IPv6DualStack)) if err != nil { return err } @@ -204,7 +205,7 @@ func coreDNSAddon(cfg *kubeadmapi.ClusterConfiguration, client clientset.Interfa return errors.Wrap(err, "error when parsing CoreDNS configMap template") } - dnsip, err := kubeadmconstants.GetDNSIP(cfg.Networking.ServiceSubnet) + dnsip, err := kubeadmconstants.GetDNSIP(cfg.Networking.ServiceSubnet, features.Enabled(cfg.FeatureGates, features.IPv6DualStack)) if err != nil { return err } diff --git a/cmd/kubeadm/app/phases/addons/dns/dns_test.go b/cmd/kubeadm/app/phases/addons/dns/dns_test.go index a13cde7b764..97be7f8d073 100644 --- a/cmd/kubeadm/app/phases/addons/dns/dns_test.go +++ b/cmd/kubeadm/app/phases/addons/dns/dns_test.go @@ -150,21 +150,36 @@ func TestCompileManifests(t *testing.T) { func TestGetDNSIP(t *testing.T) { var tests = []struct { name, svcSubnet, expectedDNSIP string + isDualStack bool }{ { name: "subnet mask 12", svcSubnet: "10.96.0.0/12", expectedDNSIP: "10.96.0.10", + isDualStack: false, }, { name: "subnet mask 26", svcSubnet: "10.87.116.64/26", expectedDNSIP: "10.87.116.74", + isDualStack: false, + }, + { + name: "dual-stack ipv4 primary, subnet mask 26", + svcSubnet: "10.87.116.64/26,fd03::/112", + expectedDNSIP: "10.87.116.74", + isDualStack: true, + }, + { + name: "dual-stack ipv6 primary, subnet mask 112", + svcSubnet: "fd03::/112,10.87.116.64/26", + expectedDNSIP: "fd03::a", + isDualStack: true, }, } for _, rt := range tests { t.Run(rt.name, func(t *testing.T) { - dnsIP, err := kubeadmconstants.GetDNSIP(rt.svcSubnet) + dnsIP, err := kubeadmconstants.GetDNSIP(rt.svcSubnet, rt.isDualStack) if err != nil { t.Fatalf("couldn't get dnsIP : %v", err) } diff --git a/cmd/kubeadm/app/preflight/checks.go b/cmd/kubeadm/app/preflight/checks.go index 21e2694d35f..a787e3e9ad9 100644 --- a/cmd/kubeadm/app/preflight/checks.go +++ b/cmd/kubeadm/app/preflight/checks.go @@ -894,10 +894,12 @@ func RunInitNodeChecks(execer utilsexec.Interface, cfg *kubeadmapi.InitConfigura FileAvailableCheck{Path: kubeadmconstants.GetStaticPodFilepath(kubeadmconstants.KubeScheduler, manifestsDir)}, FileAvailableCheck{Path: kubeadmconstants.GetStaticPodFilepath(kubeadmconstants.Etcd, manifestsDir)}, HTTPProxyCheck{Proto: "https", Host: cfg.LocalAPIEndpoint.AdvertiseAddress}, - HTTPProxyCIDRCheck{Proto: "https", CIDR: cfg.Networking.ServiceSubnet}, } - - cidrs := strings.Split(cfg.Networking.PodSubnet, ",") + cidrs := strings.Split(cfg.Networking.ServiceSubnet, ",") + for _, cidr := range cidrs { + checks = append(checks, HTTPProxyCIDRCheck{Proto: "https", CIDR: cidr}) + } + cidrs = strings.Split(cfg.Networking.PodSubnet, ",") for _, cidr := range cidrs { checks = append(checks, HTTPProxyCIDRCheck{Proto: "https", CIDR: cidr}) } diff --git a/cmd/kubeadm/app/util/pkiutil/BUILD b/cmd/kubeadm/app/util/pkiutil/BUILD index e4cc53efcfa..09b28062edd 100644 --- a/cmd/kubeadm/app/util/pkiutil/BUILD +++ b/cmd/kubeadm/app/util/pkiutil/BUILD @@ -23,8 +23,8 @@ go_library( deps = [ "//cmd/kubeadm/app/apis/kubeadm:go_default_library", "//cmd/kubeadm/app/constants:go_default_library", + "//cmd/kubeadm/app/features:go_default_library", "//cmd/kubeadm/app/util:go_default_library", - "//pkg/registry/core/service/ipallocator:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library", "//staging/src/k8s.io/client-go/util/cert:go_default_library", "//staging/src/k8s.io/client-go/util/keyutil:go_default_library", diff --git a/cmd/kubeadm/app/util/pkiutil/pki_helpers.go b/cmd/kubeadm/app/util/pkiutil/pki_helpers.go index 38ce2cace03..c5ccaeabbf7 100644 --- a/cmd/kubeadm/app/util/pkiutil/pki_helpers.go +++ b/cmd/kubeadm/app/util/pkiutil/pki_helpers.go @@ -40,8 +40,8 @@ import ( "k8s.io/client-go/util/keyutil" kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm" kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" + "k8s.io/kubernetes/cmd/kubeadm/app/features" kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util" - "k8s.io/kubernetes/pkg/registry/core/service/ipallocator" ) const ( @@ -357,15 +357,9 @@ func GetAPIServerAltNames(cfg *kubeadmapi.InitConfiguration) (*certutil.AltNames cfg.LocalAPIEndpoint.AdvertiseAddress) } - // internal IP address for the API server - _, svcSubnet, err := net.ParseCIDR(cfg.Networking.ServiceSubnet) + internalAPIServerVirtualIP, err := kubeadmconstants.GetAPIServerVirtualIP(cfg.Networking.ServiceSubnet, features.Enabled(cfg.FeatureGates, features.IPv6DualStack)) if err != nil { - return nil, errors.Wrapf(err, "error parsing CIDR %q", cfg.Networking.ServiceSubnet) - } - - internalAPIServerVirtualIP, err := ipallocator.GetIndexedIP(svcSubnet, 1) - if err != nil { - return nil, errors.Wrapf(err, "unable to get first IP address from the given CIDR (%s)", svcSubnet.String()) + return nil, errors.Wrapf(err, "unable to get first IP address from the given CIDR: %v", cfg.Networking.ServiceSubnet) } // create AltNames with defaults DNSNames/IPs