From a12baf6117b405171f1965f638d6423082c4d84b Mon Sep 17 00:00:00 2001 From: mdemirhan Date: Fri, 8 Jun 2018 17:24:57 -0700 Subject: [PATCH 1/3] * Changed the default installation to allow traffic going outside without getting terminated by Istio's sidecar. * Refactored config map informer in route controller to use a shared factory and reuse the same factory within the revision controller as well. NOTE: k8sflag with Dynamic setting was tested; however it was very inconsistent in getting updates to the config map, missing more than half of the updates. Because of that, ConfigMap informers are used to get changes in the config map. --- DEVELOPMENT.md | 6 + cmd/controller/main.go | 11 +- config/config-network.yaml | 52 +++++ pkg/controller/configuration/configuration.go | 4 +- pkg/controller/controller.go | 24 +-- pkg/controller/names.go | 4 + pkg/controller/revision/ela_pod.go | 44 +++- pkg/controller/revision/network_config.go | 55 +++++ .../revision/network_config_test.go | 67 ++++++ pkg/controller/revision/revision.go | 61 +++++- pkg/controller/revision/revision_test.go | 198 +++++++++++++++--- pkg/controller/route/queueing_test.go | 2 +- pkg/controller/route/route.go | 34 +-- pkg/controller/route/route_test.go | 51 +++-- pkg/controller/service/service.go | 4 +- 15 files changed, 511 insertions(+), 106 deletions(-) create mode 100644 config/config-network.yaml create mode 100644 pkg/controller/revision/network_config.go create mode 100644 pkg/controller/revision/network_config_test.go diff --git a/DEVELOPMENT.md b/DEVELOPMENT.md index e9059e9d7191..7701ac26d949 100644 --- a/DEVELOPMENT.md +++ b/DEVELOPMENT.md @@ -127,6 +127,12 @@ kubectl apply -f ./third_party/config/build/release.yaml This step includes building Knative Serving, creating and pushing developer images and deploying them to your Kubernetes cluster. +First, edit [config-network.yaml](config/config-network.yaml) as instructed within the file. +If this file is edited and deployed after Knative Serving installation, the changes in it will be +effective only for newly created revisions. + +Next, run: + ```shell ko apply -f config/ ``` diff --git a/cmd/controller/main.go b/cmd/controller/main.go index a6ea1c58a8fc..23f1f47087cc 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -22,6 +22,8 @@ import ( "net/http" "time" + "github.com/knative/serving/pkg" + "github.com/josephburnett/k8sflag/pkg/k8sflag" "github.com/knative/serving/pkg/controller" "github.com/knative/serving/pkg/logging" @@ -128,6 +130,8 @@ func main() { kubeInformerFactory := kubeinformers.NewSharedInformerFactory(kubeClient, time.Second*30) elaInformerFactory := informers.NewSharedInformerFactory(elaClient, time.Second*30) buildInformerFactory := buildinformers.NewSharedInformerFactory(buildClient, time.Second*30) + servingSystemInformerFactory := kubeinformers.NewFilteredSharedInformerFactory(kubeClient, + time.Minute*5, pkg.GetServingSystemNamespace(), nil) revControllerConfig := revision.ControllerConfig{ AutoscaleConcurrencyQuantumOfTime: autoscaleConcurrencyQuantumOfTime, @@ -148,14 +152,17 @@ func main() { // Add new controllers to this array. controllers := []controller.Interface{ configuration.NewController(kubeClient, elaClient, buildClient, kubeInformerFactory, elaInformerFactory, cfg, logger), - revision.NewController(kubeClient, elaClient, kubeInformerFactory, elaInformerFactory, buildInformerFactory, cfg, &revControllerConfig, logger), - route.NewController(kubeClient, elaClient, kubeInformerFactory, elaInformerFactory, cfg, autoscaleEnableScaleToZero, logger), + revision.NewController(kubeClient, elaClient, kubeInformerFactory, elaInformerFactory, + buildInformerFactory, servingSystemInformerFactory, cfg, &revControllerConfig, logger), + route.NewController(kubeClient, elaClient, kubeInformerFactory, elaInformerFactory, + servingSystemInformerFactory, cfg, autoscaleEnableScaleToZero, logger), service.NewController(kubeClient, elaClient, kubeInformerFactory, elaInformerFactory, cfg, logger), } go kubeInformerFactory.Start(stopCh) go elaInformerFactory.Start(stopCh) go buildInformerFactory.Start(stopCh) + go servingSystemInformerFactory.Start(stopCh) // Start all of the controllers. for _, ctrlr := range controllers { diff --git a/config/config-network.yaml b/config/config-network.yaml new file mode 100644 index 000000000000..c22165223365 --- /dev/null +++ b/config/config-network.yaml @@ -0,0 +1,52 @@ +# Copyright 2018 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +apiVersion: v1 +kind: ConfigMap +metadata: + name: config-network + namespace: knative-serving-system +data: + # Specifies the IP ranges that Istio sidecar will intercept. + # Replace this with the IP ranges of your cluster. + # Separate multiple entries with a comma. + # Example: "10.4.0.0/14,10.7.240.0/20" + # + # If set to "*" Istio will intercept all traffic within + # the cluster as well as traffic that is going outside the cluster. + # Traffic going outside the cluster will be blocked unless + # necessary egress rules are created. + # + # If omitted or set to "", value of global.proxy.includeIPRanges + # provided at Istio deployment time is used. In default Knative serving + # deployment, global.proxy.includeIPRanges value is set to "*". + # + # If an invalid value is used, "" is used. + # + # If valid set of IP address ranges are put into this value, + # Istio will no longer intercept traffic going to IP addresses + # outside the provided ranges and there is no need to specify + # egress rules. + # + # To determine the IP ranges of your cluster: + # IBM Cloud Private: cat cluster/config.yaml | grep service_cluster_ip_range + # IBM Cloud Kubernetes Service: "172.30.0.0/16,172.20.0.0/16,10.10.10.0/24" + # Google Container Engine (GKE): gcloud container clusters describe XXXXXXX --zone=XXXXXX | grep -e clusterIpv4Cidr -e servicesIpv4Cidr + # Azure Container Service(ACS): "10.244.0.0/16,10.240.0.0/16" + # Minikube: "10.0.0.1/24" + # + # For more information, visit + # https://istio.io/docs/tasks/traffic-management/egress/ + # + istio.sidecar.includeOutboundIPRanges: "*" diff --git a/pkg/controller/configuration/configuration.go b/pkg/controller/configuration/configuration.go index 8b1ed10642a1..eb1bd151a110 100644 --- a/pkg/controller/configuration/configuration.go +++ b/pkg/controller/configuration/configuration.go @@ -73,8 +73,8 @@ func NewController( revisionInformer := elaInformerFactory.Serving().V1alpha1().Revisions() controller := &Controller{ - Base: controller.NewBase(kubeClientSet, elaClientSet, kubeInformerFactory, - elaInformerFactory, informer.Informer(), controllerAgentName, "Configurations", logger), + Base: controller.NewBase(kubeClientSet, elaClientSet, + informer.Informer(), controllerAgentName, "Configurations", logger), buildClientSet: buildClientSet, lister: informer.Lister(), synced: informer.Informer().HasSynced, diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 6e722789259e..be27a1046998 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -22,13 +22,11 @@ import ( clientset "github.com/knative/serving/pkg/client/clientset/versioned" elascheme "github.com/knative/serving/pkg/client/clientset/versioned/scheme" - informers "github.com/knative/serving/pkg/client/informers/externalversions" "github.com/knative/serving/pkg/logging/logkey" "go.uber.org/zap" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" - kubeinformers "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" typedcorev1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -57,14 +55,6 @@ type Base struct { // ElaClientSet allows us to configure Ela objects ElaClientSet clientset.Interface - // KubeInformerFactory provides shared informers for resources - // in all known API group versions - KubeInformerFactory kubeinformers.SharedInformerFactory - - // ElaInformerFactory provides shared informers for resources - // in all known API group versions - ElaInformerFactory informers.SharedInformerFactory - // Recorder is an event recorder for recording Event resources to the // Kubernetes API. Recorder record.EventRecorder @@ -89,8 +79,6 @@ type Base struct { func NewBase( kubeClientSet kubernetes.Interface, elaClientSet clientset.Interface, - kubeInformerFactory kubeinformers.SharedInformerFactory, - elaInformerFactory informers.SharedInformerFactory, informer cache.SharedIndexInformer, controllerAgentName string, workQueueName string, @@ -107,13 +95,11 @@ func NewBase( recorder := eventBroadcaster.NewRecorder(scheme.Scheme, corev1.EventSource{Component: controllerAgentName}) base := &Base{ - KubeClientSet: kubeClientSet, - ElaClientSet: elaClientSet, - KubeInformerFactory: kubeInformerFactory, - ElaInformerFactory: elaInformerFactory, - Recorder: recorder, - WorkQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), workQueueName), - Logger: logger, + KubeClientSet: kubeClientSet, + ElaClientSet: elaClientSet, + Recorder: recorder, + WorkQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), workQueueName), + Logger: logger, } // Set up an event handler for when the resource types of interest change diff --git a/pkg/controller/names.go b/pkg/controller/names.go index 67d178dcf409..b1d1f1f5677b 100644 --- a/pkg/controller/names.go +++ b/pkg/controller/names.go @@ -34,6 +34,10 @@ func GetDomainConfigMapName() string { return "config-domain" } +func GetNetworkConfigMapName() string { + return "config-network" +} + // Various functions for naming the resources for consistency func GetElaNamespaceName(ns string) string { // We create resources in the same namespace as the Knative Serving resources by default. diff --git a/pkg/controller/revision/ela_pod.go b/pkg/controller/revision/ela_pod.go index 6da6058cfb54..0fd64ffe700d 100644 --- a/pkg/controller/revision/ela_pod.go +++ b/pkg/controller/revision/ela_pod.go @@ -17,6 +17,11 @@ limitations under the License. package revision import ( + "net" + "strings" + + "go.uber.org/zap" + "github.com/knative/serving/pkg/apis/serving/v1alpha1" "github.com/knative/serving/pkg/controller" "github.com/knative/serving/pkg/queue" @@ -34,8 +39,9 @@ const ( queueContainerCPU = "25m" fluentdContainerCPU = "75m" - fluentdConfigMapVolumeName = "configmap" - varLogVolumeName = "varlog" + fluentdConfigMapVolumeName = "configmap" + varLogVolumeName = "varlog" + istioOutboundIPRangeAnnotation = "traffic.sidecar.istio.io/includeOutboundIPRanges" ) func hasHTTPPath(p *corev1.Probe) bool { @@ -183,7 +189,8 @@ func MakeElaPodSpec( } // MakeElaDeployment creates a deployment. -func MakeElaDeployment(u *v1alpha1.Revision, namespace string) *appsv1.Deployment { +func MakeElaDeployment(logger *zap.SugaredLogger, u *v1alpha1.Revision, namespace string, + networkConfig *NetworkConfig) *appsv1.Deployment { rollingUpdateConfig := appsv1.RollingUpdateDeployment{ MaxUnavailable: &elaPodMaxUnavailable, MaxSurge: &elaPodMaxSurge, @@ -192,6 +199,23 @@ func MakeElaDeployment(u *v1alpha1.Revision, namespace string) *appsv1.Deploymen podTemplateAnnotations := MakeElaResourceAnnotations(u) podTemplateAnnotations[sidecarIstioInjectAnnotation] = "true" + // Inject the IP ranges for istio sidecar configuration. + // We will inject this value only if all of the following is true: + // - the config map contains a non-empty value + // - the user doesn't specify this annotation in their configuration + // - configured values are valid CIDR notation IP addresses + // If these conditions are not met, this value will be left untouched. + // * is a special value that is accepted as a valid. + if _, ok := podTemplateAnnotations[istioOutboundIPRangeAnnotation]; !ok { + if len(networkConfig.IstioOutboundIPRanges) > 0 { + if err := validateOutboundIPRanges(networkConfig.IstioOutboundIPRanges); err != nil { + logger.Errorf("Failed to parse IP ranges %v. Not setting the annotation. Error: %v", networkConfig.IstioOutboundIPRanges, err) + } else { + podTemplateAnnotations[istioOutboundIPRangeAnnotation] = networkConfig.IstioOutboundIPRanges + } + } + } + return &appsv1.Deployment{ ObjectMeta: meta_v1.ObjectMeta{ Name: controller.GetRevisionDeploymentName(u), @@ -215,3 +239,17 @@ func MakeElaDeployment(u *v1alpha1.Revision, namespace string) *appsv1.Deploymen }, } } + +func validateOutboundIPRanges(s string) error { + // * is a valid value + if s == "*" { + return nil + } + cidrs := strings.Split(s, ",") + for _, cidr := range cidrs { + if _, _, err := net.ParseCIDR(cidr); err != nil { + return err + } + } + return nil +} diff --git a/pkg/controller/revision/network_config.go b/pkg/controller/revision/network_config.go new file mode 100644 index 000000000000..63ab892d40ee --- /dev/null +++ b/pkg/controller/revision/network_config.go @@ -0,0 +1,55 @@ +/* +Copyright 2018 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package revision + +import ( + "github.com/knative/serving/pkg" + + "github.com/knative/serving/pkg/controller" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes" +) + +const ( + // IstioOutboundIPRangesKey is the name of the configuration entry + // that specifies Istio outbound ip ranges. + IstioOutboundIPRangesKey = "istio.sidecar.includeOutboundIPRanges" +) + +// NetworkConfig contains the networking configuration defined in the +// network config map. +type NetworkConfig struct { + // IstioOutboundIPRange specifies the IP ranges to intercept + // by Istio sidecar. + IstioOutboundIPRanges string +} + +// NewNetworkConfig creates a DomainConfig by reading the domain configmap from +// the supplied client. +func NewNetworkConfig(kubeClient kubernetes.Interface) *NetworkConfig { + m, err := kubeClient.CoreV1().ConfigMaps(pkg.GetServingSystemNamespace()).Get(controller.GetNetworkConfigMapName(), metav1.GetOptions{}) + if err != nil { + return &NetworkConfig{} + } + return NewNetworkConfigFromConfigMap(m) +} + +// NewNetworkConfigFromConfigMap creates a NewNetworkConfig from the supplied ConfigMap +func NewNetworkConfigFromConfigMap(configMap *corev1.ConfigMap) *NetworkConfig { + return &NetworkConfig{IstioOutboundIPRanges: configMap.Data[IstioOutboundIPRangesKey]} +} diff --git a/pkg/controller/revision/network_config_test.go b/pkg/controller/revision/network_config_test.go new file mode 100644 index 000000000000..6e6de79d34f9 --- /dev/null +++ b/pkg/controller/revision/network_config_test.go @@ -0,0 +1,67 @@ +/* +Copyright 2018 Google LLC. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package revision + +import ( + "testing" + + "github.com/knative/serving/pkg" + "github.com/knative/serving/pkg/controller" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + fakekubeclientset "k8s.io/client-go/kubernetes/fake" +) + +func TestNewConfigMissingConfigMap(t *testing.T) { + c := NewNetworkConfig(fakekubeclientset.NewSimpleClientset()) + if len(c.IstioOutboundIPRanges) > 0 { + t.Error("Expected an empty value when config map doesn't exist.") + } +} + +func TestNewConfigNoEntry(t *testing.T) { + kubeClient := fakekubeclientset.NewSimpleClientset() + kubeClient.CoreV1().ConfigMaps(pkg.GetServingSystemNamespace()).Create(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: pkg.GetServingSystemNamespace(), + Name: controller.GetNetworkConfigMapName(), + }, + }) + c := NewNetworkConfig(fakekubeclientset.NewSimpleClientset()) + if len(c.IstioOutboundIPRanges) > 0 { + t.Error("Expected an empty value when config map doesn't have the entry.") + } +} + +func TestNewConfig(t *testing.T) { + kubeClient := fakekubeclientset.NewSimpleClientset() + want := "10.10.10.10/12" + kubeClient.CoreV1().ConfigMaps(pkg.GetServingSystemNamespace()).Create(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: pkg.GetServingSystemNamespace(), + Name: controller.GetNetworkConfigMapName(), + }, + Data: map[string]string{ + IstioOutboundIPRangesKey: want, + "bar.com": "selector:\n app: bar\n version: beta", + }, + }) + c := NewNetworkConfig(kubeClient) + if c.IstioOutboundIPRanges != want { + t.Errorf("Want %v, got %v", want, c.IstioOutboundIPRanges) + } +} diff --git a/pkg/controller/revision/revision.go b/pkg/controller/revision/revision.go index cf453ab7b3a5..5ab745d2bfcc 100644 --- a/pkg/controller/revision/revision.go +++ b/pkg/controller/revision/revision.go @@ -22,6 +22,7 @@ import ( "net/http" "reflect" "strings" + "sync" "time" "github.com/knative/serving/pkg" @@ -99,8 +100,9 @@ type Controller struct { resolver resolver - // don't start the workers until endpoints cache have been synced + // don't start the workers until informers are synced endpointsSynced cache.InformerSynced + configMapSynced cache.InformerSynced // enableVarLogCollection dedicates whether to set up a fluentd sidecar to // collect logs under /var/log/. @@ -108,6 +110,11 @@ type Controller struct { // controllerConfig includes the configurations for the controller controllerConfig *ControllerConfig + + // networkConfig could change over time and access to it + // must go through networkConfigMutex + networkConfig *NetworkConfig + networkConfigMutex sync.Mutex } // ControllerConfig includes the configurations for the controller. @@ -162,6 +169,7 @@ func NewController( kubeInformerFactory kubeinformers.SharedInformerFactory, elaInformerFactory informers.SharedInformerFactory, buildInformerFactory buildinformers.SharedInformerFactory, + servingSystemInformerFactory kubeinformers.SharedInformerFactory, config *rest.Config, controllerConfig *ControllerConfig, logger *zap.SugaredLogger) controller.Interface { @@ -170,16 +178,19 @@ func NewController( informer := elaInformerFactory.Serving().V1alpha1().Revisions() endpointsInformer := kubeInformerFactory.Core().V1().Endpoints() deploymentInformer := kubeInformerFactory.Apps().V1().Deployments() + configMapInformer := servingSystemInformerFactory.Core().V1().ConfigMaps().Informer() controller := &Controller{ - Base: controller.NewBase(kubeClientSet, elaClientSet, kubeInformerFactory, - elaInformerFactory, informer.Informer(), controllerAgentName, "Revisions", logger), + Base: controller.NewBase(kubeClientSet, elaClientSet, + informer.Informer(), controllerAgentName, "Revisions", logger), lister: informer.Lister(), synced: informer.Informer().HasSynced, endpointsSynced: endpointsInformer.Informer().HasSynced, + configMapSynced: configMapInformer.HasSynced, buildtracker: &buildTracker{builds: map[key]set{}}, resolver: &digestResolver{client: kubeClientSet, transport: http.DefaultTransport}, controllerConfig: controllerConfig, + networkConfig: NewNetworkConfig(kubeClientSet), } controller.Logger.Info("Setting up event handlers") @@ -200,6 +211,12 @@ func NewController( UpdateFunc: controller.updateDeploymentProgressEvent, }) + configMapInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ + AddFunc: controller.addConfigMapEvent, + UpdateFunc: controller.updateConfigMapEvent, + DeleteFunc: controller.deleteConfigMapEvent, + }) + return controller } @@ -762,7 +779,7 @@ func (c *Controller) reconcileDeployment(ctx context.Context, rev *v1alpha1.Revi // Create a single pod so that it gets created before deployment->RS to try to speed // things up podSpec := MakeElaPodSpec(rev, c.controllerConfig) - deployment := MakeElaDeployment(rev, ns) + deployment := MakeElaDeployment(logger, rev, ns, c.getNetworkConfig()) deployment.OwnerReferences = append(deployment.OwnerReferences, *controllerRef) deployment.Spec.Template.Spec = *podSpec @@ -1035,3 +1052,39 @@ func lookupServiceOwner(endpoint *corev1.Endpoints) string { } return "" } + +func (c *Controller) addConfigMapEvent(obj interface{}) { + configMap := obj.(*corev1.ConfigMap) + if configMap.Namespace != pkg.GetServingSystemNamespace() || configMap.Name != controller.GetNetworkConfigMapName() { + return + } + + c.Logger.Infof("Network config map is added or updated: %v", configMap) + newNetworkConfig := NewNetworkConfigFromConfigMap(configMap) + c.Logger.Infof("IstioOutboundIPRanges: %v", newNetworkConfig.IstioOutboundIPRanges) + c.setNetworkConfig(newNetworkConfig) +} + +func (c *Controller) updateConfigMapEvent(old, new interface{}) { + c.addConfigMapEvent(new) +} + +func (c *Controller) deleteConfigMapEvent(obj interface{}) { + configMap := obj.(*corev1.ConfigMap) + if configMap.Namespace != pkg.GetServingSystemNamespace() || configMap.Name != controller.GetNetworkConfigMapName() { + return + } + c.setNetworkConfig(&NetworkConfig{}) +} + +func (c *Controller) getNetworkConfig() *NetworkConfig { + c.networkConfigMutex.Lock() + defer c.networkConfigMutex.Unlock() + return c.networkConfig +} + +func (c *Controller) setNetworkConfig(cfg *NetworkConfig) { + c.networkConfigMutex.Lock() + defer c.networkConfigMutex.Unlock() + c.networkConfig = cfg +} diff --git a/pkg/controller/revision/revision_test.go b/pkg/controller/revision/revision_test.go index 441f23d4327e..a611c1f10e0e 100644 --- a/pkg/controller/revision/revision_test.go +++ b/pkg/controller/revision/revision_test.go @@ -226,7 +226,8 @@ func newTestControllerWithConfig(t *testing.T, controllerConfig *ControllerConfi elaClient *fakeclientset.Clientset, controller *Controller, kubeInformer kubeinformers.SharedInformerFactory, - elaInformer informers.SharedInformerFactory) { + elaInformer informers.SharedInformerFactory, + servingSystemInformer kubeinformers.SharedInformerFactory) { // Create fake clients kubeClient = fakekubeclientset.NewSimpleClientset() @@ -238,6 +239,7 @@ func newTestControllerWithConfig(t *testing.T, controllerConfig *ControllerConfi kubeInformer = kubeinformers.NewSharedInformerFactory(kubeClient, 0) buildInformer := buildinformers.NewSharedInformerFactory(buildClient, 0) elaInformer = informers.NewSharedInformerFactory(elaClient, 0) + servingSystemInformer = kubeinformers.NewFilteredSharedInformerFactory(kubeClient, 0, pkg.GetServingSystemNamespace(), nil) controller = NewController( kubeClient, @@ -245,6 +247,7 @@ func newTestControllerWithConfig(t *testing.T, controllerConfig *ControllerConfi kubeInformer, elaInformer, buildInformer, + servingSystemInformer, &rest.Config{}, controllerConfig, zap.NewNop().Sugar(), @@ -261,7 +264,8 @@ func newTestController(t *testing.T, elaObjects ...runtime.Object) ( elaClient *fakeclientset.Clientset, controller *Controller, kubeInformer kubeinformers.SharedInformerFactory, - elaInformer informers.SharedInformerFactory) { + elaInformer informers.SharedInformerFactory, + servingSystemInformer kubeinformers.SharedInformerFactory) { testControllerConfig := getTestControllerConfig() return newTestControllerWithConfig(t, &testControllerConfig, elaObjects...) } @@ -274,13 +278,15 @@ func newRunningTestController(t *testing.T, elaObjects ...runtime.Object) ( elaInformer informers.SharedInformerFactory, stopCh chan struct{}) { - kubeClient, _, elaClient, controller, kubeInformer, elaInformer = newTestController(t, elaObjects...) + var servingSystemInformer kubeinformers.SharedInformerFactory + kubeClient, _, elaClient, controller, kubeInformer, elaInformer, servingSystemInformer = newTestController(t, elaObjects...) // Start the informers. This must happen after the call to NewController, // otherwise there are no informers to be started. stopCh = make(chan struct{}) kubeInformer.Start(stopCh) elaInformer.Start(stopCh) + servingSystemInformer.Start(stopCh) // Run the controller. go func() { @@ -327,7 +333,7 @@ func (r *fixedResolver) Resolve(deploy *appsv1.Deployment) error { func TestCreateRevCreatesStuff(t *testing.T) { controllerConfig := getTestControllerConfig() - kubeClient, _, elaClient, controller, _, elaInformer := newTestControllerWithConfig(t, &controllerConfig) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestControllerWithConfig(t, &controllerConfig) // Resolve image references to this "digest" digest := "foo@sha256:deadbeef" @@ -623,7 +629,7 @@ func (r *errorResolver) Resolve(deploy *appsv1.Deployment) error { } func TestResolutionFailed(t *testing.T) { - _, _, elaClient, controller, _, elaInformer := newTestController(t) + _, _, elaClient, controller, _, elaInformer, _ := newTestController(t) // Unconditionally return this error during resolution. errorMessage := "I am the expected error message, hear me ROAR!" @@ -663,7 +669,7 @@ func TestResolutionFailed(t *testing.T) { func TestCreateRevDoesNotSetUpFluentdSidecarIfVarLogCollectionDisabled(t *testing.T) { controllerConfig := getTestControllerConfig() controllerConfig.EnableVarLogCollection = false - kubeClient, _, elaClient, controller, _, elaInformer := newTestControllerWithConfig(t, &controllerConfig) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestControllerWithConfig(t, &controllerConfig) rev := getTestRevision() config := getTestConfiguration() rev.OwnerReferences = append( @@ -704,7 +710,7 @@ func TestCreateRevDoesNotSetUpFluentdSidecarIfVarLogCollectionDisabled(t *testin } func TestCreateRevUpdateConfigMap_NewData(t *testing.T) { - kubeClient, _, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() fluentdConfigSource := makeFullFluentdConfig(testFluentdSidecarOutputConfig) @@ -733,7 +739,7 @@ func TestCreateRevUpdateConfigMap_NewData(t *testing.T) { } func TestCreateRevUpdateConfigMap_NewRevOwnerReference(t *testing.T) { - kubeClient, _, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() revRef := *newRevisionNonControllerRef(rev) oldRev := getTestRevision() @@ -769,7 +775,7 @@ func TestCreateRevUpdateConfigMap_NewRevOwnerReference(t *testing.T) { func TestCreateRevWithWithLoggingURL(t *testing.T) { controllerConfig := getTestControllerConfig() controllerConfig.LoggingURLTemplate = "http://logging.test.com?filter=${REVISION_UID}" - _, _, elaClient, controller, _, elaInformer := newTestControllerWithConfig(t, &controllerConfig) + _, _, elaClient, controller, _, elaInformer, _ := newTestControllerWithConfig(t, &controllerConfig) revClient := elaClient.ServingV1alpha1().Revisions(testNamespace) rev := getTestRevision() @@ -789,7 +795,7 @@ func TestCreateRevWithWithLoggingURL(t *testing.T) { func TestUpdateRevWithWithUpdatedLoggingURL(t *testing.T) { controllerConfig := getTestControllerConfig() controllerConfig.LoggingURLTemplate = "http://old-logging.test.com?filter=${REVISION_UID}" - _, _, elaClient, controller, _, elaInformer := newTestControllerWithConfig(t, &controllerConfig) + _, _, elaClient, controller, _, elaInformer, _ := newTestControllerWithConfig(t, &controllerConfig) revClient := elaClient.ServingV1alpha1().Revisions(testNamespace) rev := getTestRevision() @@ -811,7 +817,7 @@ func TestUpdateRevWithWithUpdatedLoggingURL(t *testing.T) { } func TestCreateRevPreservesAppLabel(t *testing.T) { - kubeClient, _, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() rev.Labels[appLabelKey] = "app-label-that-should-stay-unchanged" elaClient.ServingV1alpha1().Revisions(testNamespace).Create(rev) @@ -877,7 +883,7 @@ func TestCreateRevPreservesAppLabel(t *testing.T) { } func TestCreateRevWithBuildNameWaits(t *testing.T) { - _, buildClient, elaClient, controller, _, elaInformer := newTestController(t) + _, buildClient, elaClient, controller, _, elaInformer, _ := newTestController(t) revClient := elaClient.ServingV1alpha1().Revisions(testNamespace) bld := &buildv1alpha1.Build{ @@ -926,7 +932,7 @@ func TestCreateRevWithBuildNameWaits(t *testing.T) { } func TestCreateRevWithFailedBuildNameFails(t *testing.T) { - kubeClient, buildClient, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, buildClient, elaClient, controller, _, elaInformer, _ := newTestController(t) revClient := elaClient.ServingV1alpha1().Revisions(testNamespace) reason := "Foo" @@ -1009,7 +1015,7 @@ func TestCreateRevWithFailedBuildNameFails(t *testing.T) { } func TestCreateRevWithCompletedBuildNameCompletes(t *testing.T) { - kubeClient, buildClient, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, buildClient, elaClient, controller, _, elaInformer, _ := newTestController(t) revClient := elaClient.ServingV1alpha1().Revisions(testNamespace) h := NewHooks() @@ -1089,7 +1095,7 @@ func TestCreateRevWithCompletedBuildNameCompletes(t *testing.T) { } func TestCreateRevWithInvalidBuildNameFails(t *testing.T) { - _, buildClient, elaClient, controller, _, elaInformer := newTestController(t) + _, buildClient, elaClient, controller, _, elaInformer, _ := newTestController(t) revClient := elaClient.ServingV1alpha1().Revisions(testNamespace) reason := "Foo" @@ -1162,7 +1168,7 @@ func TestCreateRevWithInvalidBuildNameFails(t *testing.T) { } func TestCreateRevWithProgressDeadlineSecondsStuff(t *testing.T) { - kubeClient, _, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestController(t) revClient := elaClient.ServingV1alpha1().Revisions(testNamespace) var testProgressDeadlineSeconds int32 = 10 @@ -1210,7 +1216,7 @@ func TestCreateRevWithProgressDeadlineSecondsStuff(t *testing.T) { } func TestMarkRevReadyUponEndpointBecomesReady(t *testing.T) { - kubeClient, _, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestController(t) revClient := elaClient.ServingV1alpha1().Revisions(testNamespace) rev := getTestRevision() @@ -1269,7 +1275,7 @@ func TestMarkRevReadyUponEndpointBecomesReady(t *testing.T) { } func TestDoNotUpdateRevIfRevIsAlreadyReady(t *testing.T) { - _, _, elaClient, controller, _, elaInformer := newTestController(t) + _, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() // Mark the revision already ready. rev.Status.Conditions = []v1alpha1.RevisionCondition{ @@ -1297,7 +1303,7 @@ func TestDoNotUpdateRevIfRevIsAlreadyReady(t *testing.T) { } func TestDoNotUpdateRevIfRevIsMarkedAsFailed(t *testing.T) { - _, _, elaClient, controller, _, elaInformer := newTestController(t) + _, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() // Mark the revision already ready. rev.Status.Conditions = []v1alpha1.RevisionCondition{ @@ -1330,7 +1336,7 @@ func TestDoNotUpdateRevIfRevIsMarkedAsFailed(t *testing.T) { } func TestMarkRevAsFailedIfEndpointHasNoAddressesAfterSomeDuration(t *testing.T) { - _, _, elaClient, controller, _, elaInformer := newTestController(t) + _, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() creationTime := time.Now().Add(-10 * time.Minute) @@ -1372,7 +1378,7 @@ func TestMarkRevAsFailedIfEndpointHasNoAddressesAfterSomeDuration(t *testing.T) } func TestAuxiliaryEndpointDoesNotUpdateRev(t *testing.T) { - _, _, elaClient, controller, _, elaInformer := newTestController(t) + _, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() createRevision(elaClient, elaInformer, controller, rev) @@ -1392,7 +1398,7 @@ func TestAuxiliaryEndpointDoesNotUpdateRev(t *testing.T) { } func TestActiveToRetiredRevisionDeletesStuff(t *testing.T) { - kubeClient, _, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() // Create revision and verify that the k8s resources are created as @@ -1419,7 +1425,7 @@ func TestActiveToRetiredRevisionDeletesStuff(t *testing.T) { } func TestActiveToReserveRevisionDeletesStuff(t *testing.T) { - kubeClient, _, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() // Create revision and verify that the k8s resources are created as @@ -1445,7 +1451,7 @@ func TestActiveToReserveRevisionDeletesStuff(t *testing.T) { } func TestRetiredToActiveRevisionCreatesStuff(t *testing.T) { - kubeClient, _, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() // Create revision. The k8s resources should not be created. @@ -1472,7 +1478,7 @@ func TestRetiredToActiveRevisionCreatesStuff(t *testing.T) { } func TestReserveToActiveRevisionCreatesStuff(t *testing.T) { - kubeClient, _, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestController(t) rev := getTestRevision() // Create revision. The k8s resources should not be created. @@ -1497,3 +1503,145 @@ func TestReserveToActiveRevisionCreatesStuff(t *testing.T) { t.Fatalf("Couldn't get ela deployment: %v", err) } } + +func TestIstioOutboundIPRangesInjection(t *testing.T) { + var annotations map[string]string + + validList := []string{ + "10.10.10.0/24", // Valid single outbound IP range + "10.10.10.0/24,10.240.10.0/14,192.192.10.0/16", // Valid multiple outbound IP ranges + "*", + } + for _, want := range validList { + annotations = getPodAnnotationsForConfig(t, want, "", false, false) + if got := annotations[istioOutboundIPRangeAnnotation]; want != got { + t.Fatalf("%v annotation expected to be %v, but is %v.", istioOutboundIPRangeAnnotation, want, got) + } + } + + invalidList := []string{ + "", // Empty input should generate no annotation + "10.10.10.10/33", // Invalid outbound IP range + "10.10.10.10/12,invalid", // Some valid, some invalid ranges + "10.10.10.10/12,-1.1.1.1/10", + ",", + ",,", + ", ,", + "*,", + "*,*", + } + for _, invalid := range invalidList { + annotations = getPodAnnotationsForConfig(t, invalid, "", false, false) + if got, ok := annotations[istioOutboundIPRangeAnnotation]; ok { + t.Fatalf("Expected to have no %v annotation for invalid option %v. But found value %v", istioOutboundIPRangeAnnotation, invalid, got) + } + } + + // Configuration has an annotation override - its value must be preserved + want := "10.240.10.0/14" + annotations = getPodAnnotationsForConfig(t, "", want, false, false) + if got := annotations[istioOutboundIPRangeAnnotation]; got != want { + t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) + } + annotations = getPodAnnotationsForConfig(t, "10.10.10.0/24", want, false, false) + if got := annotations[istioOutboundIPRangeAnnotation]; got != want { + t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) + } + + // Delete the config map + want = "10.240.10.0/14" + annotations = getPodAnnotationsForConfig(t, "", want, true, false) + if got := annotations[istioOutboundIPRangeAnnotation]; got != want { + t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) + } + annotations = getPodAnnotationsForConfig(t, "", "", true, false) + if got, ok := annotations[istioOutboundIPRangeAnnotation]; ok { + t.Fatalf("Expected to have no %v annotation. But found value %v", istioOutboundIPRangeAnnotation, got) + } + + // Update a random config map in serving namespace + want = "10.240.10.0/14" + annotations = getPodAnnotationsForConfig(t, "", want, false, true) + if got := annotations[istioOutboundIPRangeAnnotation]; got != want { + t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) + } + annotations = getPodAnnotationsForConfig(t, "10.10.10.0/24", want, false, true) + if got := annotations[istioOutboundIPRangeAnnotation]; got != want { + t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) + } + want = "10.10.10.0/24" + annotations = getPodAnnotationsForConfig(t, want, "", false, true) + if got := annotations[istioOutboundIPRangeAnnotation]; got != want { + t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) + } +} + +func getPodAnnotationsForConfig(t *testing.T, configMapValue string, configAnnotationOverride string, deleteConfigMap bool, updateRandomConfigMap bool) map[string]string { + controllerConfig := getTestControllerConfig() + kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestControllerWithConfig(t, &controllerConfig) + + // Resolve image references to this "digest" + digest := "foo@sha256:deadbeef" + controller.resolver = &fixedResolver{digest} + if deleteConfigMap { + controller.deleteConfigMapEvent(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrl.GetNetworkConfigMapName(), + Namespace: pkg.GetServingSystemNamespace(), + }, + Data: map[string]string{ + IstioOutboundIPRangesKey: configMapValue, + }}) + } else { + controller.updateConfigMapEvent(nil, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: ctrl.GetNetworkConfigMapName(), + Namespace: pkg.GetServingSystemNamespace(), + }, + Data: map[string]string{ + IstioOutboundIPRangesKey: configMapValue, + }}) + } + + if updateRandomConfigMap { + controller.updateConfigMapEvent(nil, + &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "Someotherfile", + Namespace: pkg.GetServingSystemNamespace(), + }, + Data: map[string]string{ + IstioOutboundIPRangesKey: "11.11.11.11/24", + }}) + controller.deleteConfigMapEvent(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Name: "Someotherfile2", + Namespace: pkg.GetServingSystemNamespace(), + }, + Data: map[string]string{ + IstioOutboundIPRangesKey: "11.11.11.11/24", + }}) + + } + + rev := getTestRevision() + config := getTestConfiguration() + if len(configAnnotationOverride) > 0 { + rev.ObjectMeta.Annotations = map[string]string{istioOutboundIPRangeAnnotation: configAnnotationOverride} + } + + rev.OwnerReferences = append( + rev.OwnerReferences, + *ctrl.NewConfigurationControllerRef(config), + ) + + createRevision(elaClient, elaInformer, controller, rev) + + expectedDeploymentName := fmt.Sprintf("%s-deployment", rev.Name) + deployment, err := kubeClient.AppsV1().Deployments(testNamespace).Get(expectedDeploymentName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Couldn't get ela deployment: %v", err) + } + return deployment.Spec.Template.ObjectMeta.Annotations +} diff --git a/pkg/controller/route/queueing_test.go b/pkg/controller/route/queueing_test.go index 98878448c6de..2aaf58ffe64a 100644 --- a/pkg/controller/route/queueing_test.go +++ b/pkg/controller/route/queueing_test.go @@ -52,7 +52,7 @@ func TestNewRouteCallsSyncHandler(t *testing.T) { // because ObjectTracker doesn't fire watches in the 1.9 client. When we // upgrade to 1.10 we can remove the config argument here and instead use the // Create() method. - kubeClient, _, _, _, _, stopCh := newRunningTestController(t, rev, route) + kubeClient, _, _, _, _, _, stopCh := newRunningTestController(t, rev, route) defer close(stopCh) h := NewHooks() diff --git a/pkg/controller/route/route.go b/pkg/controller/route/route.go index 636182035d98..a986faa1c774 100644 --- a/pkg/controller/route/route.go +++ b/pkg/controller/route/route.go @@ -22,7 +22,6 @@ import ( "fmt" "reflect" "sync" - "time" "github.com/knative/serving/pkg" @@ -33,7 +32,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/runtime" kubeinformers "k8s.io/client-go/informers" - coreinformers "k8s.io/client-go/informers/core/v1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" @@ -90,10 +88,9 @@ type Controller struct { lister listers.RouteLister synced cache.InformerSynced - // don't start the workers until configuration cache have been synced - configSynced cache.InformerSynced - - configMapInformer cache.SharedIndexInformer + // don't start the workers until informers are synced + configSynced cache.InformerSynced + configMapSynced cache.InformerSynced // Domain configuration could change over time and access to domainConfig // must go through domainConfigMutex @@ -114,6 +111,7 @@ func NewController( elaClientSet clientset.Interface, kubeInformerFactory kubeinformers.SharedInformerFactory, elaInformerFactory informers.SharedInformerFactory, + servingSystemInformerFactory kubeinformers.SharedInformerFactory, config *rest.Config, enableScaleToZero *k8sflag.BoolFlag, logger *zap.SugaredLogger) controller.Interface { @@ -123,20 +121,7 @@ func NewController( informer := elaInformerFactory.Serving().V1alpha1().Routes() configInformer := elaInformerFactory.Serving().V1alpha1().Configurations() ingressInformer := kubeInformerFactory.Extensions().V1beta1().Ingresses() - - // An alternate to below is to create a shared informer factory - // using NewFilteredSharedInformerFactory and use it to create - // shared informers using it and share the informer across multiple callers, - // but this case is quite specific and isn't too common to be shared. - // If it turns our that more than once caller needs it, we should bump this up - // to cmd/main.go and create a shared informer factory. - // The resync period is set to 15 minutes, because we don't really need to - // keep reading domain config map every 30 seconds like we do with our other - // informers. If we somehow miss to update the config map, 15 minute seems - // like a reasonable delay to reconcile it back as domain configuration - // should only change very few times if ever throughout the lifetime of a cluster. - configMapInformer := coreinformers.NewFilteredConfigMapInformer(kubeClientSet, pkg.GetServingSystemNamespace(), - time.Minute*15, cache.Indexers{cache.NamespaceIndex: cache.MetaNamespaceIndexFunc}, nil) + configMapInformer := servingSystemInformerFactory.Core().V1().ConfigMaps().Informer() domainConfig, err := NewDomainConfig(kubeClientSet) if err != nil { @@ -146,12 +131,12 @@ func NewController( // No need to lock domainConfigMutex yet since the informers that can modify // domainConfig haven't started yet. controller := &Controller{ - Base: controller.NewBase(kubeClientSet, elaClientSet, kubeInformerFactory, - elaInformerFactory, informer.Informer(), controllerAgentName, "Routes", logger), + Base: controller.NewBase(kubeClientSet, elaClientSet, + informer.Informer(), controllerAgentName, "Routes", logger), lister: informer.Lister(), synced: informer.Informer().HasSynced, configSynced: configInformer.Informer().HasSynced, - configMapInformer: configMapInformer, + configMapSynced: configMapInformer.HasSynced, domainConfig: domainConfig, enableScaleToZero: enableScaleToZero, } @@ -176,8 +161,7 @@ func NewController( // is closed, at which point it will shutdown the workqueue and wait for // workers to finish processing their current work items. func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) error { - go c.configMapInformer.Run(stopCh) - return c.RunController(threadiness, stopCh, []cache.InformerSynced{c.synced, c.configSynced, c.configMapInformer.HasSynced}, + return c.RunController(threadiness, stopCh, []cache.InformerSynced{c.synced, c.configSynced, c.configMapSynced}, c.updateRouteEvent, "Route") } diff --git a/pkg/controller/route/route_test.go b/pkg/controller/route/route_test.go index 84daea25cf15..c2885df6c112 100644 --- a/pkg/controller/route/route_test.go +++ b/pkg/controller/route/route_test.go @@ -168,7 +168,8 @@ func newTestController(t *testing.T, elaObjects ...runtime.Object) ( elaClient *fakeclientset.Clientset, controller *Controller, kubeInformer kubeinformers.SharedInformerFactory, - elaInformer informers.SharedInformerFactory) { + elaInformer informers.SharedInformerFactory, + servingSystemInformer kubeinformers.SharedInformerFactory) { // Create fake clients kubeClient = fakekubeclientset.NewSimpleClientset() @@ -189,12 +190,14 @@ func newTestController(t *testing.T, elaObjects ...runtime.Object) ( // resync period to zero, disabling it. kubeInformer = kubeinformers.NewSharedInformerFactory(kubeClient, 0) elaInformer = informers.NewSharedInformerFactory(elaClient, 0) + servingSystemInformer = kubeinformers.NewFilteredSharedInformerFactory(kubeClient, 0, pkg.GetServingSystemNamespace(), nil) controller = NewController( kubeClient, elaClient, kubeInformer, elaInformer, + servingSystemInformer, &rest.Config{}, k8sflag.Bool("enable-scale-to-zero", false), testLogger, @@ -209,15 +212,17 @@ func newRunningTestController(t *testing.T, elaObjects ...runtime.Object) ( controller *Controller, kubeInformer kubeinformers.SharedInformerFactory, elaInformer informers.SharedInformerFactory, + servingSystemInformer kubeinformers.SharedInformerFactory, stopCh chan struct{}) { - kubeClient, elaClient, controller, kubeInformer, elaInformer = newTestController(t, elaObjects...) + kubeClient, elaClient, controller, kubeInformer, elaInformer, servingSystemInformer = newTestController(t, elaObjects...) // Start the informers. This must happen after the call to NewController, // otherwise there are no informers to be started. stopCh = make(chan struct{}) kubeInformer.Start(stopCh) elaInformer.Start(stopCh) + servingSystemInformer.Start(stopCh) // Run the controller. go func() { @@ -230,7 +235,7 @@ func newRunningTestController(t *testing.T, elaObjects ...runtime.Object) ( } func TestCreateRouteCreatesStuff(t *testing.T) { - kubeClient, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, elaClient, controller, _, elaInformer, _ := newTestController(t) h := NewHooks() // Look for the events. Events are delivered asynchronously so we need to use @@ -356,7 +361,7 @@ func TestCreateRouteCreatesStuff(t *testing.T) { // Test the only revision in the route is in Reserve (inactive) serving status. func TestCreateRouteForOneReserveRevision(t *testing.T) { - kubeClient, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, elaClient, controller, _, elaInformer, _ := newTestController(t) controller.enableScaleToZero = k8sflag.Bool("enable-scale-to-zero", true) h := NewHooks() @@ -451,7 +456,7 @@ func TestCreateRouteForOneReserveRevision(t *testing.T) { } func TestCreateRouteFromConfigsWithMultipleRevs(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) // A configuration and associated revision. Normally the revision would be // created by the configuration controller. @@ -538,7 +543,7 @@ func TestCreateRouteFromConfigsWithMultipleRevs(t *testing.T) { } func TestCreateRouteWithMultipleTargets(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) // A standalone revision rev := getTestRevision("test-rev") elaClient.ServingV1alpha1().Revisions(testNamespace).Create(rev) @@ -619,7 +624,7 @@ func TestCreateRouteWithMultipleTargets(t *testing.T) { // Test one out of multiple target revisions is in Reserve serving state. func TestCreateRouteWithOneTargetReserve(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) controller.enableScaleToZero = k8sflag.Bool("enable-scale-to-zero", true) // A standalone inactive revision @@ -704,7 +709,7 @@ func TestCreateRouteWithOneTargetReserve(t *testing.T) { } func TestCreateRouteWithDuplicateTargets(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) // A standalone revision rev := getTestRevision("test-rev") @@ -821,7 +826,7 @@ func TestCreateRouteWithDuplicateTargets(t *testing.T) { } func TestCreateRouteWithNamedTargets(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) // A standalone revision rev := getTestRevision("test-rev") elaClient.ServingV1alpha1().Revisions(testNamespace).Create(rev) @@ -966,7 +971,7 @@ func TestCreateRouteWithNamedTargets(t *testing.T) { } func TestCreateRouteDeletesOutdatedRouteRules(t *testing.T) { - _, elaClient, controller, _, _ := newTestController(t) + _, elaClient, controller, _, _, _ := newTestController(t) config := getTestConfiguration() rev := getTestRevisionForConfig(config) route := getTestRouteWithTrafficTargets( @@ -1034,7 +1039,7 @@ func TestCreateRouteDeletesOutdatedRouteRules(t *testing.T) { } func TestSetLabelToConfigurationDirectlyConfigured(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) config := getTestConfiguration() rev := getTestRevisionForConfig(config) route := getTestRouteWithTrafficTargets( @@ -1066,7 +1071,7 @@ func TestSetLabelToConfigurationDirectlyConfigured(t *testing.T) { } func TestSetLabelToRevisionDirectlyConfigured(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) config := getTestConfiguration() rev := getTestRevisionForConfig(config) route := getTestRouteWithTrafficTargets( @@ -1122,7 +1127,7 @@ func TestSetLabelToRevisionDirectlyConfigured(t *testing.T) { } func TestSetLabelToConfigurationAndRevisionIndirectlyConfigured(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) config := getTestConfiguration() rev := getTestRevisionForConfig(config) route := getTestRouteWithTrafficTargets( @@ -1169,7 +1174,7 @@ func TestSetLabelToConfigurationAndRevisionIndirectlyConfigured(t *testing.T) { } func TestCreateRouteWithInvalidConfigurationShouldReturnError(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) config := getTestConfiguration() rev := getTestRevisionForConfig(config) route := getTestRouteWithTrafficTargets( @@ -1219,7 +1224,7 @@ func sortConditions(a, b v1alpha1.RouteCondition) bool { } func TestCreateRouteRevisionMissingCondition(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) config := getTestConfiguration() rev := getTestRevisionForConfig(config) route := getTestRouteWithTrafficTargets( @@ -1268,7 +1273,7 @@ func TestCreateRouteRevisionMissingCondition(t *testing.T) { } func TestCreateRouteConfigurationMissingCondition(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) config := getTestConfiguration() rev := getTestRevisionForConfig(config) route := getTestRouteWithTrafficTargets( @@ -1317,7 +1322,7 @@ func TestCreateRouteConfigurationMissingCondition(t *testing.T) { } func TestSetLabelNotChangeConfigurationAndRevisionLabelIfLabelExists(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) config := getTestConfiguration() rev := getTestRevisionForConfig(config) route := getTestRouteWithTrafficTargets( @@ -1362,7 +1367,7 @@ func TestSetLabelNotChangeConfigurationAndRevisionLabelIfLabelExists(t *testing. } func TestDeleteLabelOfConfigurationAndRevisionWhenUnconfigured(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) route := getTestRouteWithTrafficTargets([]v1alpha1.TrafficTarget{}) config := getTestConfiguration() // Set a route label in configuration which is expected to be deleted. @@ -1406,7 +1411,7 @@ func TestDeleteLabelOfConfigurationAndRevisionWhenUnconfigured(t *testing.T) { } func TestUpdateRouteDomainWhenRouteLabelChanges(t *testing.T) { - kubeClient, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, elaClient, controller, _, elaInformer, _ := newTestController(t) route := getTestRouteWithTrafficTargets([]v1alpha1.TrafficTarget{}) routeClient := elaClient.ServingV1alpha1().Routes(route.Namespace) ingressClient := kubeClient.ExtensionsV1beta1().Ingresses(route.Namespace) @@ -1455,7 +1460,7 @@ func TestUpdateRouteDomainWhenRouteLabelChanges(t *testing.T) { } func TestUpdateRouteWhenConfigurationChanges(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) routeClient := elaClient.ServingV1alpha1().Routes(testNamespace) config := getTestConfiguration() @@ -1530,7 +1535,7 @@ func TestUpdateRouteWhenConfigurationChanges(t *testing.T) { } func TestAddConfigurationEventNotUpdateAnythingIfHasNoLatestReady(t *testing.T) { - _, elaClient, controller, _, elaInformer := newTestController(t) + _, elaClient, controller, _, elaInformer, _ := newTestController(t) config := getTestConfiguration() rev := getTestRevisionForConfig(config) route := getTestRouteWithTrafficTargets( @@ -1572,7 +1577,7 @@ func TestAddConfigurationEventNotUpdateAnythingIfHasNoLatestReady(t *testing.T) // Test route when we do not use activator, and then use activator. func TestUpdateIngressEventUpdateRouteStatus(t *testing.T) { - kubeClient, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, elaClient, controller, _, elaInformer, _ := newTestController(t) // A standalone revision rev := getTestRevision("test-rev") @@ -1635,7 +1640,7 @@ func TestUpdateIngressEventUpdateRouteStatus(t *testing.T) { } func TestUpdateDomainConfigMap(t *testing.T) { - kubeClient, elaClient, controller, _, elaInformer := newTestController(t) + kubeClient, elaClient, controller, _, elaInformer, _ := newTestController(t) route := getTestRouteWithTrafficTargets([]v1alpha1.TrafficTarget{}) routeClient := elaClient.ServingV1alpha1().Routes(route.Namespace) ingressClient := kubeClient.Extensions().Ingresses(route.Namespace) diff --git a/pkg/controller/service/service.go b/pkg/controller/service/service.go index 69acffeb845f..02df1d68f46a 100644 --- a/pkg/controller/service/service.go +++ b/pkg/controller/service/service.go @@ -87,8 +87,8 @@ func NewController( informer := elaInformerFactory.Serving().V1alpha1().Services() controller := &Controller{ - Base: controller.NewBase(kubeClientSet, elaClientSet, kubeInformerFactory, - elaInformerFactory, informer.Informer(), controllerAgentName, "Revisions", logger), + Base: controller.NewBase(kubeClientSet, elaClientSet, + informer.Informer(), controllerAgentName, "Revisions", logger), lister: informer.Lister(), synced: informer.Informer().HasSynced, } From cb13a4ef852f93b57863dc224edd42e29b89ea90 Mon Sep 17 00:00:00 2001 From: mdemirhan Date: Sat, 9 Jun 2018 13:34:29 -0700 Subject: [PATCH 2/3] * Require network config map to exist before starting the controller --- pkg/controller/revision/network_config.go | 6 +- .../revision/network_config_test.go | 17 ++++-- pkg/controller/revision/revision.go | 18 +++--- pkg/controller/revision/revision_test.go | 58 ++++++------------- pkg/controller/route/route.go | 2 +- 5 files changed, 39 insertions(+), 62 deletions(-) diff --git a/pkg/controller/revision/network_config.go b/pkg/controller/revision/network_config.go index 63ab892d40ee..df737e129437 100644 --- a/pkg/controller/revision/network_config.go +++ b/pkg/controller/revision/network_config.go @@ -41,12 +41,12 @@ type NetworkConfig struct { // NewNetworkConfig creates a DomainConfig by reading the domain configmap from // the supplied client. -func NewNetworkConfig(kubeClient kubernetes.Interface) *NetworkConfig { +func NewNetworkConfig(kubeClient kubernetes.Interface) (*NetworkConfig, error) { m, err := kubeClient.CoreV1().ConfigMaps(pkg.GetServingSystemNamespace()).Get(controller.GetNetworkConfigMapName(), metav1.GetOptions{}) if err != nil { - return &NetworkConfig{} + return nil, err } - return NewNetworkConfigFromConfigMap(m) + return NewNetworkConfigFromConfigMap(m), nil } // NewNetworkConfigFromConfigMap creates a NewNetworkConfig from the supplied ConfigMap diff --git a/pkg/controller/revision/network_config_test.go b/pkg/controller/revision/network_config_test.go index 6e6de79d34f9..972320168d47 100644 --- a/pkg/controller/revision/network_config_test.go +++ b/pkg/controller/revision/network_config_test.go @@ -27,9 +27,9 @@ import ( ) func TestNewConfigMissingConfigMap(t *testing.T) { - c := NewNetworkConfig(fakekubeclientset.NewSimpleClientset()) - if len(c.IstioOutboundIPRanges) > 0 { - t.Error("Expected an empty value when config map doesn't exist.") + _, err := NewNetworkConfig(fakekubeclientset.NewSimpleClientset()) + if err == nil { + t.Error("Expected an error value when config map doesn't exist.") } } @@ -41,8 +41,10 @@ func TestNewConfigNoEntry(t *testing.T) { Name: controller.GetNetworkConfigMapName(), }, }) - c := NewNetworkConfig(fakekubeclientset.NewSimpleClientset()) - if len(c.IstioOutboundIPRanges) > 0 { + c, err := NewNetworkConfig(kubeClient) + if err != nil { + t.Errorf("Didn't expect an error but got %v", err) + } else if len(c.IstioOutboundIPRanges) > 0 { t.Error("Expected an empty value when config map doesn't have the entry.") } } @@ -60,7 +62,10 @@ func TestNewConfig(t *testing.T) { "bar.com": "selector:\n app: bar\n version: beta", }, }) - c := NewNetworkConfig(kubeClient) + c, err := NewNetworkConfig(kubeClient) + if err != nil { + t.Errorf("Didn't expect an error but got %v", err) + } if c.IstioOutboundIPRanges != want { t.Errorf("Want %v, got %v", want, c.IstioOutboundIPRanges) } diff --git a/pkg/controller/revision/revision.go b/pkg/controller/revision/revision.go index 5ab745d2bfcc..b9a62f656256 100644 --- a/pkg/controller/revision/revision.go +++ b/pkg/controller/revision/revision.go @@ -180,6 +180,11 @@ func NewController( deploymentInformer := kubeInformerFactory.Apps().V1().Deployments() configMapInformer := servingSystemInformerFactory.Core().V1().ConfigMaps().Informer() + networkConfig, err := NewNetworkConfig(kubeClientSet) + if err != nil { + logger.Fatalf("Error loading network config: %v", err) + } + controller := &Controller{ Base: controller.NewBase(kubeClientSet, elaClientSet, informer.Informer(), controllerAgentName, "Revisions", logger), @@ -190,7 +195,7 @@ func NewController( buildtracker: &buildTracker{builds: map[key]set{}}, resolver: &digestResolver{client: kubeClientSet, transport: http.DefaultTransport}, controllerConfig: controllerConfig, - networkConfig: NewNetworkConfig(kubeClientSet), + networkConfig: networkConfig, } controller.Logger.Info("Setting up event handlers") @@ -214,7 +219,6 @@ func NewController( configMapInformer.AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: controller.addConfigMapEvent, UpdateFunc: controller.updateConfigMapEvent, - DeleteFunc: controller.deleteConfigMapEvent, }) return controller @@ -225,7 +229,7 @@ func NewController( // is closed, at which point it will shutdown the workqueue and wait for // workers to finish processing their current work items. func (c *Controller) Run(threadiness int, stopCh <-chan struct{}) error { - return c.RunController(threadiness, stopCh, []cache.InformerSynced{c.synced, c.endpointsSynced}, + return c.RunController(threadiness, stopCh, []cache.InformerSynced{c.synced, c.endpointsSynced, c.configMapSynced}, c.syncHandler, "Revision") } @@ -1069,14 +1073,6 @@ func (c *Controller) updateConfigMapEvent(old, new interface{}) { c.addConfigMapEvent(new) } -func (c *Controller) deleteConfigMapEvent(obj interface{}) { - configMap := obj.(*corev1.ConfigMap) - if configMap.Namespace != pkg.GetServingSystemNamespace() || configMap.Name != controller.GetNetworkConfigMapName() { - return - } - c.setNetworkConfig(&NetworkConfig{}) -} - func (c *Controller) getNetworkConfig() *NetworkConfig { c.networkConfigMutex.Lock() defer c.networkConfigMutex.Unlock() diff --git a/pkg/controller/revision/revision_test.go b/pkg/controller/revision/revision_test.go index a611c1f10e0e..1e5305a5b639 100644 --- a/pkg/controller/revision/revision_test.go +++ b/pkg/controller/revision/revision_test.go @@ -234,6 +234,13 @@ func newTestControllerWithConfig(t *testing.T, controllerConfig *ControllerConfi buildClient = fakebuildclientset.NewSimpleClientset() elaClient = fakeclientset.NewSimpleClientset(elaObjects...) + kubeClient.CoreV1().ConfigMaps(pkg.GetServingSystemNamespace()).Create(&corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: pkg.GetServingSystemNamespace(), + Name: ctrl.GetNetworkConfigMapName(), + }, + }) + // Create informer factories with fake clients. The second parameter sets the // resync period to zero, disabling it. kubeInformer = kubeinformers.NewSharedInformerFactory(kubeClient, 0) @@ -1513,7 +1520,7 @@ func TestIstioOutboundIPRangesInjection(t *testing.T) { "*", } for _, want := range validList { - annotations = getPodAnnotationsForConfig(t, want, "", false, false) + annotations = getPodAnnotationsForConfig(t, want, "", false) if got := annotations[istioOutboundIPRangeAnnotation]; want != got { t.Fatalf("%v annotation expected to be %v, but is %v.", istioOutboundIPRangeAnnotation, want, got) } @@ -1531,7 +1538,7 @@ func TestIstioOutboundIPRangesInjection(t *testing.T) { "*,*", } for _, invalid := range invalidList { - annotations = getPodAnnotationsForConfig(t, invalid, "", false, false) + annotations = getPodAnnotationsForConfig(t, invalid, "", false) if got, ok := annotations[istioOutboundIPRangeAnnotation]; ok { t.Fatalf("Expected to have no %v annotation for invalid option %v. But found value %v", istioOutboundIPRangeAnnotation, invalid, got) } @@ -1539,52 +1546,41 @@ func TestIstioOutboundIPRangesInjection(t *testing.T) { // Configuration has an annotation override - its value must be preserved want := "10.240.10.0/14" - annotations = getPodAnnotationsForConfig(t, "", want, false, false) + annotations = getPodAnnotationsForConfig(t, "", want, false) if got := annotations[istioOutboundIPRangeAnnotation]; got != want { t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) } - annotations = getPodAnnotationsForConfig(t, "10.10.10.0/24", want, false, false) + annotations = getPodAnnotationsForConfig(t, "10.10.10.0/24", want, false) if got := annotations[istioOutboundIPRangeAnnotation]; got != want { t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) } - // Delete the config map - want = "10.240.10.0/14" - annotations = getPodAnnotationsForConfig(t, "", want, true, false) - if got := annotations[istioOutboundIPRangeAnnotation]; got != want { - t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) - } - annotations = getPodAnnotationsForConfig(t, "", "", true, false) - if got, ok := annotations[istioOutboundIPRangeAnnotation]; ok { - t.Fatalf("Expected to have no %v annotation. But found value %v", istioOutboundIPRangeAnnotation, got) - } - // Update a random config map in serving namespace want = "10.240.10.0/14" - annotations = getPodAnnotationsForConfig(t, "", want, false, true) + annotations = getPodAnnotationsForConfig(t, "", want, true) if got := annotations[istioOutboundIPRangeAnnotation]; got != want { t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) } - annotations = getPodAnnotationsForConfig(t, "10.10.10.0/24", want, false, true) + annotations = getPodAnnotationsForConfig(t, "10.10.10.0/24", want, true) if got := annotations[istioOutboundIPRangeAnnotation]; got != want { t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) } want = "10.10.10.0/24" - annotations = getPodAnnotationsForConfig(t, want, "", false, true) + annotations = getPodAnnotationsForConfig(t, want, "", true) if got := annotations[istioOutboundIPRangeAnnotation]; got != want { t.Fatalf("%v annotation is expected to have %v but got %v", istioOutboundIPRangeAnnotation, want, got) } } -func getPodAnnotationsForConfig(t *testing.T, configMapValue string, configAnnotationOverride string, deleteConfigMap bool, updateRandomConfigMap bool) map[string]string { +func getPodAnnotationsForConfig(t *testing.T, configMapValue string, configAnnotationOverride string, updateRandomConfigMap bool) map[string]string { controllerConfig := getTestControllerConfig() kubeClient, _, elaClient, controller, _, elaInformer, _ := newTestControllerWithConfig(t, &controllerConfig) // Resolve image references to this "digest" digest := "foo@sha256:deadbeef" controller.resolver = &fixedResolver{digest} - if deleteConfigMap { - controller.deleteConfigMapEvent(&corev1.ConfigMap{ + controller.updateConfigMapEvent(nil, + &corev1.ConfigMap{ ObjectMeta: metav1.ObjectMeta{ Name: ctrl.GetNetworkConfigMapName(), Namespace: pkg.GetServingSystemNamespace(), @@ -1592,17 +1588,6 @@ func getPodAnnotationsForConfig(t *testing.T, configMapValue string, configAnnot Data: map[string]string{ IstioOutboundIPRangesKey: configMapValue, }}) - } else { - controller.updateConfigMapEvent(nil, - &corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: ctrl.GetNetworkConfigMapName(), - Namespace: pkg.GetServingSystemNamespace(), - }, - Data: map[string]string{ - IstioOutboundIPRangesKey: configMapValue, - }}) - } if updateRandomConfigMap { controller.updateConfigMapEvent(nil, @@ -1614,15 +1599,6 @@ func getPodAnnotationsForConfig(t *testing.T, configMapValue string, configAnnot Data: map[string]string{ IstioOutboundIPRangesKey: "11.11.11.11/24", }}) - controller.deleteConfigMapEvent(&corev1.ConfigMap{ - ObjectMeta: metav1.ObjectMeta{ - Name: "Someotherfile2", - Namespace: pkg.GetServingSystemNamespace(), - }, - Data: map[string]string{ - IstioOutboundIPRangesKey: "11.11.11.11/24", - }}) - } rev := getTestRevision() diff --git a/pkg/controller/route/route.go b/pkg/controller/route/route.go index a986faa1c774..b32ca4ff2c40 100644 --- a/pkg/controller/route/route.go +++ b/pkg/controller/route/route.go @@ -125,7 +125,7 @@ func NewController( domainConfig, err := NewDomainConfig(kubeClientSet) if err != nil { - logger.Fatalf("Error loading controller config: %v", err) + logger.Fatalf("Error loading domain config: %v", err) } // No need to lock domainConfigMutex yet since the informers that can modify From 03c4037c1f2ec08f1c165185d3835a289c65fcce Mon Sep 17 00:00:00 2001 From: mdemirhan Date: Sat, 9 Jun 2018 13:46:08 -0700 Subject: [PATCH 3/3] * Address PR feedback. --- config/config-network.yaml | 2 +- pkg/controller/revision/ela_pod.go | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/config/config-network.yaml b/config/config-network.yaml index c22165223365..1dd8472378c5 100644 --- a/config/config-network.yaml +++ b/config/config-network.yaml @@ -19,7 +19,7 @@ metadata: namespace: knative-serving-system data: # Specifies the IP ranges that Istio sidecar will intercept. - # Replace this with the IP ranges of your cluster. + # Replace this with the IP ranges of your cluster (see below for some examples). # Separate multiple entries with a comma. # Example: "10.4.0.0/14,10.7.240.0/20" # diff --git a/pkg/controller/revision/ela_pod.go b/pkg/controller/revision/ela_pod.go index 0fd64ffe700d..651aacf39b13 100644 --- a/pkg/controller/revision/ela_pod.go +++ b/pkg/controller/revision/ela_pod.go @@ -200,12 +200,13 @@ func MakeElaDeployment(logger *zap.SugaredLogger, u *v1alpha1.Revision, namespac podTemplateAnnotations[sidecarIstioInjectAnnotation] = "true" // Inject the IP ranges for istio sidecar configuration. - // We will inject this value only if all of the following is true: + // We will inject this value only if all of the following are true: // - the config map contains a non-empty value - // - the user doesn't specify this annotation in their configuration + // - the user doesn't specify this annotation in configuration's pod template // - configured values are valid CIDR notation IP addresses // If these conditions are not met, this value will be left untouched. // * is a special value that is accepted as a valid. + // * intercepts calls to all IPs: in cluster as well as outside the cluster. if _, ok := podTemplateAnnotations[istioOutboundIPRangeAnnotation]; !ok { if len(networkConfig.IstioOutboundIPRanges) > 0 { if err := validateOutboundIPRanges(networkConfig.IstioOutboundIPRanges); err != nil {