diff --git a/pkg/controlplane/controller.go b/pkg/controlplane/controller.go index 0cb04944b46..087f50a2875 100644 --- a/pkg/controlplane/controller.go +++ b/pkg/controlplane/controller.go @@ -41,6 +41,7 @@ import ( servicecontroller "k8s.io/kubernetes/pkg/registry/core/service/ipallocator/controller" portallocatorcontroller "k8s.io/kubernetes/pkg/registry/core/service/portallocator/controller" "k8s.io/kubernetes/pkg/util/async" + netutils "k8s.io/utils/net" ) const ( @@ -88,10 +89,21 @@ type Controller struct { } // NewBootstrapController returns a controller for watching the core capabilities of the master -func (c *completedConfig) NewBootstrapController(legacyRESTStorage corerest.LegacyRESTStorage, serviceClient corev1client.ServicesGetter, nsClient corev1client.NamespacesGetter, eventClient corev1client.EventsGetter, readyzClient rest.Interface) *Controller { +func (c *completedConfig) NewBootstrapController(legacyRESTStorage corerest.LegacyRESTStorage, serviceClient corev1client.ServicesGetter, nsClient corev1client.NamespacesGetter, eventClient corev1client.EventsGetter, readyzClient rest.Interface) (*Controller, error) { _, publicServicePort, err := c.GenericConfig.SecureServing.HostPort() if err != nil { - klog.Fatalf("failed to get listener address: %v", err) + return nil, fmt.Errorf("failed to get listener address: %w", err) + } + + // The "kubernetes.default" Service is SingleStack based on the configured ServiceIPRange. + // If the bootstrap controller reconcile the kubernetes.default Service and Endpoints, it must + // guarantee that the Service ClusterIP and the associated Endpoints have the same IP family, or + // it will not work for clients because of the IP family mismatch. + // TODO: revisit for dual-stack https://github.com/kubernetes/enhancements/issues/2438 + if c.ExtraConfig.EndpointReconcilerType != reconcilers.NoneEndpointReconcilerType { + if netutils.IsIPv4CIDR(&c.ExtraConfig.ServiceIPRange) != netutils.IsIPv4(c.GenericConfig.PublicAddress) { + return nil, fmt.Errorf("service IP family %q must match public address family %q", c.ExtraConfig.ServiceIPRange.String(), c.GenericConfig.PublicAddress.String()) + } } systemNamespaces := []string{metav1.NamespaceSystem, metav1.NamespacePublic, corev1.NamespaceNodeLease} @@ -127,7 +139,7 @@ func (c *completedConfig) NewBootstrapController(legacyRESTStorage corerest.Lega ExtraEndpointPorts: c.ExtraConfig.ExtraEndpointPorts, PublicServicePort: publicServicePort, KubernetesServiceNodePort: c.ExtraConfig.KubernetesServiceNodePort, - } + }, nil } // PostStartHook initiates the core controller loops that must exist for bootstrapping. diff --git a/pkg/controlplane/controller_test.go b/pkg/controlplane/controller_test.go index 58a85688f65..19d1c6c790e 100644 --- a/pkg/controlplane/controller_test.go +++ b/pkg/controlplane/controller_test.go @@ -24,9 +24,13 @@ import ( discoveryv1 "k8s.io/api/discovery/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" + genericapiserver "k8s.io/apiserver/pkg/server" "k8s.io/client-go/kubernetes/fake" + corev1client "k8s.io/client-go/kubernetes/typed/core/v1" + "k8s.io/client-go/rest" core "k8s.io/client-go/testing" "k8s.io/kubernetes/pkg/controlplane/reconcilers" + corerest "k8s.io/kubernetes/pkg/registry/core/rest" netutils "k8s.io/utils/net" ) @@ -997,3 +1001,157 @@ func TestCreateOrUpdateMasterService(t *testing.T) { } } } + +func Test_completedConfig_NewBootstrapController(t *testing.T) { + + _, ipv4cidr, err := netutils.ParseCIDRSloppy("192.168.0.0/24") + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + + _, ipv6cidr, err := netutils.ParseCIDRSloppy("2001:db8::/112") + if err != nil { + t.Fatalf("Unexpected error %v", err) + } + + ipv4address := netutils.ParseIPSloppy("192.168.1.1") + ipv6address := netutils.ParseIPSloppy("2001:db8::1") + + type args struct { + legacyRESTStorage corerest.LegacyRESTStorage + serviceClient corev1client.ServicesGetter + nsClient corev1client.NamespacesGetter + eventClient corev1client.EventsGetter + readyzClient rest.Interface + } + tests := []struct { + name string + config genericapiserver.Config + extraConfig *ExtraConfig + args args + wantErr bool + }{ + { + name: "master endpoint reconciler - IPv4 families", + extraConfig: &ExtraConfig{ + EndpointReconcilerType: reconcilers.MasterCountReconcilerType, + ServiceIPRange: *ipv4cidr, + }, + config: genericapiserver.Config{ + PublicAddress: ipv4address, + SecureServing: &genericapiserver.SecureServingInfo{Listener: fakeLocalhost443Listener{}}, + }, + wantErr: false, + }, + { + name: "master endpoint reconciler - IPv6 families", + extraConfig: &ExtraConfig{ + EndpointReconcilerType: reconcilers.MasterCountReconcilerType, + ServiceIPRange: *ipv6cidr, + }, + config: genericapiserver.Config{ + PublicAddress: ipv6address, + SecureServing: &genericapiserver.SecureServingInfo{Listener: fakeLocalhost443Listener{}}, + }, + wantErr: false, + }, + { + name: "master endpoint reconciler - wrong IP families", + extraConfig: &ExtraConfig{ + EndpointReconcilerType: reconcilers.MasterCountReconcilerType, + ServiceIPRange: *ipv4cidr, + }, + config: genericapiserver.Config{ + PublicAddress: ipv6address, + SecureServing: &genericapiserver.SecureServingInfo{Listener: fakeLocalhost443Listener{}}, + }, + wantErr: true, + }, + { + name: "master endpoint reconciler - wrong IP families", + extraConfig: &ExtraConfig{ + EndpointReconcilerType: reconcilers.MasterCountReconcilerType, + ServiceIPRange: *ipv6cidr, + }, + config: genericapiserver.Config{ + PublicAddress: ipv4address, + SecureServing: &genericapiserver.SecureServingInfo{Listener: fakeLocalhost443Listener{}}, + }, + wantErr: true, + }, + { + name: "lease endpoint reconciler - IPv4 families", + extraConfig: &ExtraConfig{ + EndpointReconcilerType: reconcilers.LeaseEndpointReconcilerType, + ServiceIPRange: *ipv4cidr, + }, + config: genericapiserver.Config{ + PublicAddress: ipv4address, + SecureServing: &genericapiserver.SecureServingInfo{Listener: fakeLocalhost443Listener{}}, + }, + wantErr: false, + }, + { + name: "lease endpoint reconciler - IPv6 families", + extraConfig: &ExtraConfig{ + EndpointReconcilerType: reconcilers.LeaseEndpointReconcilerType, + ServiceIPRange: *ipv6cidr, + }, + config: genericapiserver.Config{ + PublicAddress: ipv6address, + SecureServing: &genericapiserver.SecureServingInfo{Listener: fakeLocalhost443Listener{}}, + }, + wantErr: false, + }, + { + name: "lease endpoint reconciler - wrong IP families", + extraConfig: &ExtraConfig{ + EndpointReconcilerType: reconcilers.LeaseEndpointReconcilerType, + ServiceIPRange: *ipv4cidr, + }, + config: genericapiserver.Config{ + PublicAddress: ipv6address, + SecureServing: &genericapiserver.SecureServingInfo{Listener: fakeLocalhost443Listener{}}, + }, + wantErr: true, + }, + { + name: "lease endpoint reconciler - wrong IP families", + extraConfig: &ExtraConfig{ + EndpointReconcilerType: reconcilers.LeaseEndpointReconcilerType, + ServiceIPRange: *ipv6cidr, + }, + config: genericapiserver.Config{ + PublicAddress: ipv4address, + SecureServing: &genericapiserver.SecureServingInfo{Listener: fakeLocalhost443Listener{}}, + }, + wantErr: true, + }, + { + name: "none endpoint reconciler - wrong IP families", + extraConfig: &ExtraConfig{ + EndpointReconcilerType: reconcilers.NoneEndpointReconcilerType, + ServiceIPRange: *ipv4cidr, + }, + config: genericapiserver.Config{ + PublicAddress: ipv6address, + SecureServing: &genericapiserver.SecureServingInfo{Listener: fakeLocalhost443Listener{}}, + }, + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &completedConfig{ + GenericConfig: tt.config.Complete(nil), + ExtraConfig: tt.extraConfig, + } + _, err := c.NewBootstrapController(tt.args.legacyRESTStorage, tt.args.serviceClient, tt.args.nsClient, tt.args.eventClient, tt.args.readyzClient) + if (err != nil) != tt.wantErr { + t.Errorf("completedConfig.NewBootstrapController() error = %v, wantErr %v", err, tt.wantErr) + return + } + + }) + } +} diff --git a/pkg/controlplane/instance.go b/pkg/controlplane/instance.go index 75a67a81005..5fcd716cda5 100644 --- a/pkg/controlplane/instance.go +++ b/pkg/controlplane/instance.go @@ -532,7 +532,10 @@ func (m *Instance) InstallLegacyAPI(c *completedConfig, restOptionsGetter generi controllerName := "bootstrap-controller" coreClient := corev1client.NewForConfigOrDie(c.GenericConfig.LoopbackClientConfig) - bootstrapController := c.NewBootstrapController(legacyRESTStorage, coreClient, coreClient, coreClient, coreClient.RESTClient()) + bootstrapController, err := c.NewBootstrapController(legacyRESTStorage, coreClient, coreClient, coreClient, coreClient.RESTClient()) + if err != nil { + return fmt.Errorf("error creating bootstrap controller: %v", err) + } m.GenericAPIServer.AddPostStartHookOrDie(controllerName, bootstrapController.PostStartHook) m.GenericAPIServer.AddPreShutdownHookOrDie(controllerName, bootstrapController.PreShutdownHook) diff --git a/test/integration/dualstack/dualstack_test.go b/test/integration/dualstack/dualstack_test.go index 53a68136626..46fd8839ee4 100644 --- a/test/integration/dualstack/dualstack_test.go +++ b/test/integration/dualstack/dualstack_test.go @@ -270,6 +270,8 @@ func TestCreateServiceDualStackIPv6(t *testing.T) { t.Fatalf("bad cidr: %v", err) } cfg.ExtraConfig.ServiceIPRange = *cidr + cfg.GenericConfig.PublicAddress = netutils.ParseIPSloppy("2001:db8::10") + _, s, closeFn := framework.RunAnAPIServer(cfg) defer closeFn() @@ -715,6 +717,7 @@ func TestCreateServiceDualStackIPv6IPv4(t *testing.T) { t.Fatalf("bad cidr: %v", err) } cfg.ExtraConfig.ServiceIPRange = *cidr + cfg.GenericConfig.PublicAddress = netutils.ParseIPSloppy("2001:db8::10") _, secCidr, err := netutils.ParseCIDRSloppy(secondaryServiceCIDR) if err != nil { diff --git a/test/integration/framework/controlplane_utils.go b/test/integration/framework/controlplane_utils.go index e9978e33ffe..4085c473159 100644 --- a/test/integration/framework/controlplane_utils.go +++ b/test/integration/framework/controlplane_utils.go @@ -325,7 +325,8 @@ func NewControlPlaneConfigWithOptions(opts *ControlPlaneConfigOptions) *controlp // TODO: get rid of these tests or port them to secure serving genericConfig.SecureServing = &genericapiserver.SecureServingInfo{Listener: fakeLocalhost443Listener{}} - + // if using endpoint reconciler the service subnet IP family must match the Public address + genericConfig.PublicAddress = netutils.ParseIPSloppy("10.1.1.1") err = etcdOptions.ApplyWithStorageFactoryTo(storageFactory, genericConfig) if err != nil { panic(err)