From 44d8861c316a6be83b85a819a5b0ac6f69336b80 Mon Sep 17 00:00:00 2001 From: glindsell Date: Tue, 14 Jul 2020 17:08:00 +0100 Subject: [PATCH 1/8] Add new component and endpoint labels to operator and remove old labels Signed-off-by: glindsell --- .../v1/seldondeployment_types.go | 3 + operator/controllers/labels.go | 60 ++++-- operator/controllers/labels_test.go | 193 +++++++++++++----- .../seldondeployment_controller.go | 46 +++-- .../seldondeployment_controller_test.go | 9 +- .../controllers/seldondeployment_engine.go | 17 +- .../seldondeployment_explainers.go | 11 +- .../seldondeployment_prepackaged_servers.go | 7 +- 8 files changed, 244 insertions(+), 102 deletions(-) diff --git a/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go b/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go index c94f629d5a..3079a63b3b 100644 --- a/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go +++ b/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go @@ -46,6 +46,9 @@ const ( Label_managed_by = "app.kubernetes.io/managed-by" Label_value_seldon = "seldon-core" + Label_component = "component" + Label_endpoint = "endpoint" + PODINFO_VOLUME_NAME = "seldon-podinfo" OLD_PODINFO_VOLUME_NAME = "podinfo" PODINFO_VOLUME_PATH = "/etc/podinfo" diff --git a/operator/controllers/labels.go b/operator/controllers/labels.go index f6347cb48c..26955a1e28 100644 --- a/operator/controllers/labels.go +++ b/operator/controllers/labels.go @@ -6,40 +6,72 @@ import ( corev1 "k8s.io/api/core/v1" ) -func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUnit, p machinelearningv1.PredictorSpec) { +func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec) { if pu.Type != nil { switch *pu.Type { case machinelearningv1.ROUTER: - svc.Labels[machinelearningv1.Label_router] = "true" + svc.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_router case machinelearningv1.COMBINER: - svc.Labels[machinelearningv1.Label_combiner] = "true" + svc.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_combiner case machinelearningv1.MODEL: - svc.Labels[machinelearningv1.Label_model] = "true" + svc.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_model case machinelearningv1.TRANSFORMER: - svc.Labels[machinelearningv1.Label_transformer] = "true" + svc.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_transformer case machinelearningv1.OUTPUT_TRANSFORMER: - svc.Labels[machinelearningv1.Label_output_transformer] = "true" + svc.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_output_transformer } } if p.Shadow != true && (p.Traffic >= 50 || p.Traffic == 0) { - svc.Labels[machinelearningv1.Label_default] = "true" + svc.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_default } if p.Shadow == true { - svc.Labels[machinelearningv1.Label_shadow] = "true" + svc.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_shadow } if p.Traffic < 50 && p.Traffic > 0 { - svc.Labels[machinelearningv1.Label_canary] = "true" + svc.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_canary } if !isEmptyExplainer(p.Explainer) { - svc.Labels[machinelearningv1.Label_explainer] = "true" + svc.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_explainer } svc.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon } -func addLabelsToDeployment(deploy *appsv1.Deployment, containerServiceKey, containerServiceValue string) { - deploy.ObjectMeta.Labels[containerServiceKey] = containerServiceValue - deploy.Spec.Selector.MatchLabels[containerServiceKey] = containerServiceValue - deploy.Spec.Template.ObjectMeta.Labels[containerServiceKey] = containerServiceValue +func addLabelsToDeployment(deploy *appsv1.Deployment, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec) { + if pu.Type != nil { + switch *pu.Type { + case machinelearningv1.ROUTER: + deploy.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_router + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_router + case machinelearningv1.COMBINER: + deploy.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_combiner + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_combiner + case machinelearningv1.MODEL: + deploy.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_model + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_model + case machinelearningv1.TRANSFORMER: + deploy.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_transformer + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_transformer + case machinelearningv1.OUTPUT_TRANSFORMER: + deploy.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_output_transformer + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_output_transformer + } + } + if p.Shadow != true && (p.Traffic >= 50 || p.Traffic == 0) { + deploy.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_default + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_default + } + if p.Shadow == true { + deploy.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_shadow + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_shadow + } + if p.Traffic < 50 && p.Traffic > 0 { + deploy.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_canary + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_canary + } + if !isEmptyExplainer(p.Explainer) { + deploy.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_explainer + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_explainer + } deploy.ObjectMeta.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon } diff --git a/operator/controllers/labels_test.go b/operator/controllers/labels_test.go index 44638d310c..da6d71f32c 100644 --- a/operator/controllers/labels_test.go +++ b/operator/controllers/labels_test.go @@ -1,8 +1,6 @@ package controllers import ( - "testing" - . "github.com/onsi/ginkgo" . "github.com/onsi/ginkgo/extensions/table" . "github.com/onsi/gomega" @@ -12,37 +10,135 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -func TestAddLabelsToDeployment(t *testing.T) { - g := NewGomegaWithT(t) - d := &appsv1.Deployment{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{}, - }, - Spec: appsv1.DeploymentSpec{ - Selector: &metav1.LabelSelector{ - MatchLabels: map[string]string{}, +var _ = Describe("addLabelsToDeployment", func() { + + var d *appsv1.Deployment + var p *machinelearningv1.PredictorSpec + var pu *machinelearningv1.PredictiveUnit + + BeforeEach(func() { + d = &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, }, - Template: corev1.PodTemplateSpec{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{}, + Spec: appsv1.DeploymentSpec{ + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{}, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{}, + }, }, }, - }, - } - t.Run("adds correct label to Deployment", func(t *testing.T) { - addLabelsToDeployment(d, "TestKey", "TestValue") - g.Expect(d.ObjectMeta.Labels["TestKey"]).To(Equal("TestValue")) - g.Expect(d.Spec.Selector.MatchLabels["TestKey"]).To(Equal("TestValue")) - g.Expect(d.Spec.Template.ObjectMeta.Labels["TestKey"]).To(Equal("TestValue")) - g.Expect(d.ObjectMeta.Labels[machinelearningv1.Label_managed_by]).To(Equal(machinelearningv1.Label_value_seldon)) - g.Expect(d.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_managed_by]).To(Equal(machinelearningv1.Label_value_seldon)) + } + p = &machinelearningv1.PredictorSpec{} + pu = &machinelearningv1.PredictiveUnit{} }) -} + + DescribeTable( + "Adds correct Component label to Deployment from Predictive Unit Type", + func(puType machinelearningv1.PredictiveUnitType, resultComponent, resultEndpoint string) { + pu.Type = &puType + addLabelsToDeployment(d, pu, p) + + Expect(d.Labels[machinelearningv1.Label_component]).To(Equal(resultComponent)) + Expect(d.Labels[machinelearningv1.Label_endpoint]).To(Equal(resultEndpoint)) + Expect(d.Labels[machinelearningv1.Label_managed_by]).To(Equal(machinelearningv1.Label_value_seldon)) + }, + Entry("router", machinelearningv1.ROUTER, machinelearningv1.Label_router, machinelearningv1.Label_default), + Entry("combiner", machinelearningv1.COMBINER, machinelearningv1.Label_combiner, machinelearningv1.Label_default), + Entry("model", machinelearningv1.MODEL, machinelearningv1.Label_model, machinelearningv1.Label_default), + Entry("transformer", machinelearningv1.TRANSFORMER, machinelearningv1.Label_transformer, machinelearningv1.Label_default), + Entry("output transformer", machinelearningv1.OUTPUT_TRANSFORMER, machinelearningv1.Label_output_transformer, machinelearningv1.Label_default), + ) + + DescribeTable( + "Adds correct Endpoint label to Deployment from Predictor Spec", + func(shadow bool, explainer *machinelearningv1.Explainer, traffic int, resultComponent, resultEndpoint string) { + p.Shadow = shadow + p.Explainer = explainer + p.Traffic = int32(traffic) + + // Required until https://github.com/SeldonIO/seldon-core/pull/1600 is + // merged. We should remove afterwards. + puType := machinelearningv1.MODEL + pu.Type = &puType + + addLabelsToDeployment(d, pu, p) + + Expect(d.Labels[machinelearningv1.Label_component]).To(Equal(resultComponent)) + Expect(d.Labels[machinelearningv1.Label_endpoint]).To(Equal(resultEndpoint)) + + // Check template labels for pod + Expect(d.Spec.Template.Labels[machinelearningv1.Label_component]).To(Equal(resultComponent)) + Expect(d.Spec.Template.Labels[machinelearningv1.Label_endpoint]).To(Equal(resultEndpoint)) + }, + Entry( + "default", + false, + nil, + 0, + machinelearningv1.Label_model, + machinelearningv1.Label_default, + ), + Entry( + "default with 50%% traffic", + false, + nil, + 50, + machinelearningv1.Label_model, + machinelearningv1.Label_default, + ), + Entry( + "default with 75%% traffic", + false, + nil, + 50, + machinelearningv1.Label_model, + machinelearningv1.Label_default, + ), + Entry( + "default with empty explainer", + false, + &machinelearningv1.Explainer{}, + 0, + machinelearningv1.Label_model, + machinelearningv1.Label_default, + ), + Entry( + "shadow", + true, + nil, + 0, + machinelearningv1.Label_model, + machinelearningv1.Label_shadow, + ), + Entry( + "canary", + false, + nil, + 48, + machinelearningv1.Label_model, + machinelearningv1.Label_canary, + ), + Entry( + "explainer", + false, + &machinelearningv1.Explainer{ + Type: machinelearningv1.AlibiAnchorsImageExplainer, + }, + 0, + machinelearningv1.Label_model, + machinelearningv1.Label_explainer, + ), + ) +}) var _ = Describe("addLabelsToService", func() { var svc *corev1.Service - var p machinelearningv1.PredictorSpec + var p *machinelearningv1.PredictorSpec var pu *machinelearningv1.PredictiveUnit BeforeEach(func() { @@ -51,29 +147,30 @@ var _ = Describe("addLabelsToService", func() { Labels: map[string]string{}, }, } - p = machinelearningv1.PredictorSpec{} + p = &machinelearningv1.PredictorSpec{} pu = &machinelearningv1.PredictiveUnit{} }) DescribeTable( - "Adds correct label to Service from Predictive Unit Type", - func(puType machinelearningv1.PredictiveUnitType, result string) { + "Adds correct Component label to Service from Predictive Unit Type", + func(puType machinelearningv1.PredictiveUnitType, resultComponent, resultEndpoint string) { pu.Type = &puType addLabelsToService(svc, pu, p) - Expect(svc.Labels[result]).To(Equal("true")) + Expect(svc.Labels[machinelearningv1.Label_component]).To(Equal(resultComponent)) + Expect(svc.Labels[machinelearningv1.Label_endpoint]).To(Equal(resultEndpoint)) Expect(svc.Labels[machinelearningv1.Label_managed_by]).To(Equal(machinelearningv1.Label_value_seldon)) }, - Entry("router", machinelearningv1.ROUTER, machinelearningv1.Label_router), - Entry("combiner", machinelearningv1.COMBINER, machinelearningv1.Label_combiner), - Entry("model", machinelearningv1.MODEL, machinelearningv1.Label_model), - Entry("transformer", machinelearningv1.TRANSFORMER, machinelearningv1.Label_transformer), - Entry("output transformer", machinelearningv1.OUTPUT_TRANSFORMER, machinelearningv1.Label_output_transformer), + Entry("router", machinelearningv1.ROUTER, machinelearningv1.Label_router, machinelearningv1.Label_default), + Entry("combiner", machinelearningv1.COMBINER, machinelearningv1.Label_combiner, machinelearningv1.Label_default), + Entry("model", machinelearningv1.MODEL, machinelearningv1.Label_model, machinelearningv1.Label_default), + Entry("transformer", machinelearningv1.TRANSFORMER, machinelearningv1.Label_transformer, machinelearningv1.Label_default), + Entry("output transformer", machinelearningv1.OUTPUT_TRANSFORMER, machinelearningv1.Label_output_transformer, machinelearningv1.Label_default), ) DescribeTable( "Adds correct label to Service from Predictor Spec", - func(shadow bool, explainer *machinelearningv1.Explainer, traffic int, result string, missing []string) { + func(shadow bool, explainer *machinelearningv1.Explainer, traffic int, resultComponent, resultEndpoint string) { p.Shadow = shadow p.Explainer = explainer p.Traffic = int32(traffic) @@ -85,64 +182,56 @@ var _ = Describe("addLabelsToService", func() { addLabelsToService(svc, pu, p) - Expect(svc.Labels[result]).To(Equal("true")) - for _, m := range missing { - Expect(svc.Labels).ToNot(HaveKey(m)) - } + Expect(svc.Labels[machinelearningv1.Label_component]).To(Equal(resultComponent)) + Expect(svc.Labels[machinelearningv1.Label_endpoint]).To(Equal(resultEndpoint)) }, Entry( "default", false, nil, 0, + machinelearningv1.Label_model, machinelearningv1.Label_default, - []string{machinelearningv1.Label_explainer}, ), Entry( "default with 50%% traffic", false, nil, 50, + machinelearningv1.Label_model, machinelearningv1.Label_default, - []string{machinelearningv1.Label_explainer}, ), Entry( "default with 75%% traffic", false, nil, 50, + machinelearningv1.Label_model, machinelearningv1.Label_default, - []string{machinelearningv1.Label_explainer}, ), Entry( "default with empty explainer", false, &machinelearningv1.Explainer{}, 0, + machinelearningv1.Label_model, machinelearningv1.Label_default, - []string{machinelearningv1.Label_explainer}, ), Entry( "shadow", true, nil, 0, + machinelearningv1.Label_model, machinelearningv1.Label_shadow, - []string{ - machinelearningv1.Label_explainer, - machinelearningv1.Label_default, - }, ), Entry( "canary", false, nil, 48, + machinelearningv1.Label_model, machinelearningv1.Label_canary, - []string{ - machinelearningv1.Label_explainer, - machinelearningv1.Label_default, - }, ), Entry( "explainer", @@ -151,8 +240,8 @@ var _ = Describe("addLabelsToService", func() { Type: machinelearningv1.AlibiAnchorsImageExplainer, }, 0, + machinelearningv1.Label_model, machinelearningv1.Label_explainer, - []string{}, ), ) }) diff --git a/operator/controllers/seldondeployment_controller.go b/operator/controllers/seldondeployment_controller.go index 7655720df0..f92224394b 100644 --- a/operator/controllers/seldondeployment_controller.go +++ b/operator/controllers/seldondeployment_controller.go @@ -393,13 +393,14 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S for i := 0; i < len(mlDep.Spec.Predictors); i++ { p := mlDep.Spec.Predictors[i] + pu := machinelearningv1.GetPredictiveUnit(&p.Graph, p.ComponentSpecs[0].Spec.Containers[0].Name) _, noEngine := p.Annotations[machinelearningv1.ANNOTATION_NO_ENGINE] pSvcName := machinelearningv1.GetPredictorKey(mlDep, &p) log.Info("pSvcName", "val", pSvcName) // Add engine deployment if separate _, hasSeparateEnginePod := mlDep.Spec.Annotations[machinelearningv1.ANNOTATION_SEPARATE_ENGINE] if hasSeparateEnginePod && !noEngine { - deploy, err := createEngineDeployment(mlDep, &p, pSvcName, engine_http_port, engine_grpc_port) + deploy, err := createEngineDeployment(mlDep, pu, &p, pSvcName, engine_http_port, engine_grpc_port) if err != nil { return nil, err } @@ -422,7 +423,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S if i == 0 && j == 0 { c.defaultDeploymentName = depName } - deploy := createDeploymentWithoutEngine(depName, seldonId, cSpec, &p, mlDep, securityContext) + deploy := createDeploymentWithoutEngine(depName, seldonId, cSpec, pu, &p, mlDep, securityContext) // Add HPA if needed if cSpec.HpaSpec != nil { c.hpas = append(c.hpas, createHpa(cSpec, depName, seldonId, namespace)) @@ -464,10 +465,11 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S port := int(svc.Spec.Ports[0].Port) + pu := machinelearningv1.GetPredictiveUnit(&p.Graph, p.ComponentSpecs[0].Spec.Containers[0].Name) if svc.Spec.Ports[0].Name == "grpc" { httpAllowed = false externalPorts[i] = httpGrpcPorts{httpPort: 0, grpcPort: port} - psvc, err := createPredictorService(pSvcName, seldonId, &p, mlDep, 0, port, false, log) + psvc, err := createPredictorService(pSvcName, seldonId, pu, &p, mlDep, 0, port, false, log) if err != nil { return nil, err } @@ -481,7 +483,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S } else { externalPorts[i] = httpGrpcPorts{httpPort: port, grpcPort: 0} grpcAllowed = false - psvc, err := createPredictorService(pSvcName, seldonId, &p, mlDep, port, 0, false, log) + psvc, err := createPredictorService(pSvcName, seldonId, pu, &p, mlDep, port, 0, false, log) if err != nil { return nil, err } @@ -550,7 +552,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S if grpcAllowed == false { grpcPort = 0 } - psvc, err := createPredictorService(pSvcName, seldonId, &p, mlDep, httpPort, grpcPort, false, log) + psvc, err := createPredictorService(pSvcName, seldonId, pu, &p, mlDep, httpPort, grpcPort, false, log) if err != nil { return nil, err @@ -604,20 +606,22 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S } //Creates Service for Predictor - exposed externally (ambassador or istio) -func createPredictorService(pSvcName string, seldonId string, p *machinelearningv1.PredictorSpec, +func createPredictorService(pSvcName string, seldonId string, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec, mlDep *machinelearningv1.SeldonDeployment, engine_http_port int, engine_grpc_port int, isExplainer bool, log logr.Logger) (pSvc *corev1.Service, err error) { namespace := getNamespace(mlDep) - psvc := &corev1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: pSvcName, Namespace: namespace, - Labels: map[string]string{machinelearningv1.Label_seldon_app: pSvcName, - machinelearningv1.Label_seldon_id: seldonId, machinelearningv1.Label_managed_by: machinelearningv1.Label_value_seldon}, + Labels: map[string]string{ + machinelearningv1.Label_seldon_app: pSvcName, + machinelearningv1.Label_seldon_id: seldonId, + machinelearningv1.Label_managed_by: machinelearningv1.Label_value_seldon, + }, }, Spec: corev1.ServiceSpec{ Selector: map[string]string{machinelearningv1.Label_seldon_app: pSvcName}, @@ -625,7 +629,6 @@ func createPredictorService(pSvcName string, seldonId string, p *machinelearning Type: corev1.ServiceTypeClusterIP, }, } - if isExecutorEnabled(mlDep) { if engine_http_port != 0 && len(psvc.Spec.Ports) == 0 { psvc.Spec.Ports = append(psvc.Spec.Ports, corev1.ServicePort{Protocol: corev1.ProtocolTCP, Port: int32(engine_http_port), TargetPort: intstr.FromInt(engine_http_port), Name: "http"}) @@ -643,7 +646,6 @@ func createPredictorService(pSvcName string, seldonId string, p *machinelearning psvc.Spec.Ports = append(psvc.Spec.Ports, corev1.ServicePort{Protocol: corev1.ProtocolTCP, Port: int32(engine_grpc_port), TargetPort: intstr.FromInt(engine_grpc_port), Name: "grpc"}) } } - if GetEnv("AMBASSADOR_ENABLED", "false") == "true" { psvc.Annotations = make(map[string]string) //Create top level Service @@ -653,12 +655,11 @@ func createPredictorService(pSvcName string, seldonId string, p *machinelearning } psvc.Annotations[AMBASSADOR_ANNOTATION] = ambassadorConfig } - if getAnnotation(mlDep, machinelearningv1.ANNOTATION_HEADLESS_SVC, "false") != "false" { log.Info("Creating Headless SVC") psvc.Spec.ClusterIP = "None" } - + addLabelsToService(psvc, pu, p) return psvc, err } @@ -669,7 +670,6 @@ func createContainerService(deploy *appsv1.Deployment, con *corev1.Container, c components, seldonId string) *corev1.Service { - //containerServiceKey := machinelearningv1.GetPredictorServiceNameKey(con) containerServiceKey := machinelearningv1.Label_seldon_app_svc containerServiceValue := machinelearningv1.GetContainerServiceName(mlDep.Name, p, con) pu := machinelearningv1.GetPredictiveUnit(&p.Graph, con.Name) @@ -732,8 +732,8 @@ func createContainerService(deploy *appsv1.Deployment, SessionAffinity: corev1.ServiceAffinityNone, }, } - addLabelsToService(svc, pu, p) - addLabelsToDeployment(deploy, containerServiceKey, containerServiceValue) + addLabelsToService(svc, pu, &p) + addLabelsToDeployment(deploy, pu, &p) if existingPort == nil || con.Ports == nil { con.Ports = append(con.Ports, corev1.ContainerPort{Name: portType, ContainerPort: portNum, Protocol: corev1.ProtocolTCP}) @@ -790,12 +790,16 @@ func createContainerService(deploy *appsv1.Deployment, return svc } -func createDeploymentWithoutEngine(depName string, seldonId string, seldonPodSpec *machinelearningv1.SeldonPodSpec, p *machinelearningv1.PredictorSpec, mlDep *machinelearningv1.SeldonDeployment, podSecurityContext *corev1.PodSecurityContext) *appsv1.Deployment { +func createDeploymentWithoutEngine(depName string, seldonId string, seldonPodSpec *machinelearningv1.SeldonPodSpec, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec, mlDep *machinelearningv1.SeldonDeployment, podSecurityContext *corev1.PodSecurityContext) *appsv1.Deployment { deploy := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ - Name: depName, - Namespace: getNamespace(mlDep), - Labels: map[string]string{machinelearningv1.Label_seldon_id: seldonId, "app": depName, "fluentd": "true"}, + Name: depName, + Namespace: getNamespace(mlDep), + Labels: map[string]string{ + machinelearningv1.Label_seldon_id: seldonId, + "app": depName, + "fluentd": "true", + }, Annotations: map[string]string{}, }, Spec: appsv1.DeploymentSpec{ @@ -890,6 +894,8 @@ func createDeploymentWithoutEngine(depName string, seldonId string, seldonPodSpe {Path: "annotations", FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.annotations", APIVersion: "v1"}}}, DefaultMode: &defaultMode}}}) } + addLabelsToDeployment(deploy, pu, p) + return deploy } diff --git a/operator/controllers/seldondeployment_controller_test.go b/operator/controllers/seldondeployment_controller_test.go index 123cabd369..3e030f8c41 100644 --- a/operator/controllers/seldondeployment_controller_test.go +++ b/operator/controllers/seldondeployment_controller_test.go @@ -18,6 +18,9 @@ package controllers import ( "context" + "testing" + "time" + . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" machinelearningv1 "github.com/seldonio/seldon-core/operator/apis/machinelearning.seldon.io/v1" @@ -28,8 +31,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/rand" - "testing" - "time" ) var _ = Describe("Create a Seldon Deployment", func() { @@ -869,7 +870,7 @@ func TestCreateDeploymentWithLabelsAndAnnotations(t *testing.T) { }, } - dep := createDeploymentWithoutEngine(depName, "a", instance.Spec.Predictors[0].ComponentSpecs[0], &instance.Spec.Predictors[0], instance, nil) + dep := createDeploymentWithoutEngine(depName, "a", instance.Spec.Predictors[0].ComponentSpecs[0], &instance.Spec.Predictors[0].Graph, &instance.Spec.Predictors[0], instance, nil) g.Expect(dep.Labels[labelKey1]).To(Equal(labelValue1)) g.Expect(dep.Labels[labelKey2]).To(Equal(labelValue2)) g.Expect(dep.Spec.Template.ObjectMeta.Labels[labelKey1]).To(Equal(labelValue1)) @@ -914,5 +915,5 @@ func TestCreateDeploymentWithNoLabelsAndAnnotations(t *testing.T) { }, } - _ = createDeploymentWithoutEngine(depName, "a", instance.Spec.Predictors[0].ComponentSpecs[0], &instance.Spec.Predictors[0], instance, nil) + _ = createDeploymentWithoutEngine(depName, "a", instance.Spec.Predictors[0].ComponentSpecs[0], &instance.Spec.Predictors[0].Graph, &instance.Spec.Predictors[0], instance, nil) } diff --git a/operator/controllers/seldondeployment_engine.go b/operator/controllers/seldondeployment_engine.go index 56a7ab59b7..8e8080dc56 100644 --- a/operator/controllers/seldondeployment_engine.go +++ b/operator/controllers/seldondeployment_engine.go @@ -389,7 +389,7 @@ func createEngineContainer(mlDep *machinelearningv1.SeldonDeployment, p *machine } // Create the service orchestrator. -func createEngineDeployment(mlDep *machinelearningv1.SeldonDeployment, p *machinelearningv1.PredictorSpec, seldonId string, engine_http_port, engine_grpc_port int) (*appsv1.Deployment, error) { +func createEngineDeployment(mlDep *machinelearningv1.SeldonDeployment, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec, seldonId string, engine_http_port, engine_grpc_port int) (*appsv1.Deployment, error) { var terminationGracePeriodSecs = int64(20) var defaultMode = corev1.DownwardAPIVolumeSourceDefaultMode @@ -402,9 +402,16 @@ func createEngineDeployment(mlDep *machinelearningv1.SeldonDeployment, p *machin } deploy := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ - Name: depName, - Namespace: getNamespace(mlDep), - Labels: map[string]string{machinelearningv1.Label_svc_orch: "true", machinelearningv1.Label_seldon_app: seldonId, machinelearningv1.Label_seldon_id: seldonId, "app": depName, "version": "v1", "fluentd": "true"}, + Name: depName, + Namespace: getNamespace(mlDep), + Labels: map[string]string{ + machinelearningv1.Label_svc_orch: "true", + machinelearningv1.Label_seldon_app: seldonId, + machinelearningv1.Label_seldon_id: seldonId, + "app": depName, + "version": "v1", + "fluentd": "true", + }, Annotations: mlDep.Spec.Annotations, }, Spec: appsv1.DeploymentSpec{ @@ -459,6 +466,6 @@ func createEngineDeployment(mlDep *machinelearningv1.SeldonDeployment, p *machin deploy.ObjectMeta.Labels[k] = v deploy.Spec.Template.ObjectMeta.Labels[k] = v } - + addLabelsToDeployment(deploy, pu, p) return deploy, nil } diff --git a/operator/controllers/seldondeployment_explainers.go b/operator/controllers/seldondeployment_explainers.go index 68498f971c..d8cbc5ed86 100644 --- a/operator/controllers/seldondeployment_explainers.go +++ b/operator/controllers/seldondeployment_explainers.go @@ -18,12 +18,15 @@ package controllers import ( "fmt" - "k8s.io/client-go/kubernetes" "sort" "strconv" "strings" + "k8s.io/client-go/kubernetes" + "encoding/json" + "os" + "github.com/go-logr/logr" machinelearningv1 "github.com/seldonio/seldon-core/operator/apis/machinelearning.seldon.io/v1" "github.com/seldonio/seldon-core/operator/constants" @@ -33,7 +36,6 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" - "os" ) const ( @@ -80,6 +82,7 @@ func getExplainerConfigsFromMap(configMap *corev1.ConfigMap) (*ExplainerConfig, func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonDeployment, p *machinelearningv1.PredictorSpec, c *components, pSvcName string, podSecurityContect *corev1.PodSecurityContext, log logr.Logger) error { if !isEmptyExplainer(p.Explainer) { + pu := machinelearningv1.GetPredictiveUnit(&p.Graph, p.ComponentSpecs[0].Spec.Containers[0].Name) seldonId := machinelearningv1.GetSeldonDeploymentName(mlDep) @@ -201,7 +204,7 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD Containers: []corev1.Container{explainerContainer}, }} - deploy := createDeploymentWithoutEngine(depName, seldonId, &seldonPodSpec, p, mlDep, podSecurityContect) + deploy := createDeploymentWithoutEngine(depName, seldonId, &seldonPodSpec, pu, p, mlDep, podSecurityContect) if p.Explainer.ModelUri != "" { var err error @@ -222,7 +225,7 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD c.deployments = append(c.deployments, deploy) // Use seldondeployment name dash explainer as the external service name. This should allow canarying. - eSvc, err := createPredictorService(eSvcName, seldonId, p, mlDep, httpPort, grpcPort, true, log) + eSvc, err := createPredictorService(eSvcName, seldonId, pu, p, mlDep, httpPort, grpcPort, true, log) if err != nil { return err } diff --git a/operator/controllers/seldondeployment_prepackaged_servers.go b/operator/controllers/seldondeployment_prepackaged_servers.go index 95ef57ed3c..092ba0bad9 100644 --- a/operator/controllers/seldondeployment_prepackaged_servers.go +++ b/operator/controllers/seldondeployment_prepackaged_servers.go @@ -19,14 +19,15 @@ package controllers import ( "encoding/json" "fmt" + "strconv" + "strings" + machinelearningv1 "github.com/seldonio/seldon-core/operator/apis/machinelearning.seldon.io/v1" "github.com/seldonio/seldon-core/operator/constants" "github.com/seldonio/seldon-core/operator/utils" appsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" "k8s.io/client-go/kubernetes" - "strconv" - "strings" ) const ( @@ -282,7 +283,7 @@ func (pi *PrePackedInitialiser) createStandaloneModelServers(mlDep *machinelearn // might not be a Deployment yet - if so we have to create one if deploy == nil { seldonId := machinelearningv1.GetSeldonDeploymentName(mlDep) - deploy = createDeploymentWithoutEngine(depName, seldonId, sPodSpec, p, mlDep, podSecurityContext) + deploy = createDeploymentWithoutEngine(depName, seldonId, sPodSpec, pu, p, mlDep, podSecurityContext) } serverConfig := machinelearningv1.GetPrepackServerConfig(string(*pu.Implementation)) From cf55b7102a0a44b358f82457452d0792a0e5be94 Mon Sep 17 00:00:00 2001 From: glindsell Date: Fri, 17 Jul 2020 15:22:51 +0100 Subject: [PATCH 2/8] Add container service keys and values back in Signed-off-by: glindsell --- operator/controllers/seldondeployment_controller.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/operator/controllers/seldondeployment_controller.go b/operator/controllers/seldondeployment_controller.go index 364733ae8b..2b07d4b63c 100644 --- a/operator/controllers/seldondeployment_controller.go +++ b/operator/controllers/seldondeployment_controller.go @@ -735,6 +735,9 @@ func createContainerService(deploy *appsv1.Deployment, } addLabelsToService(svc, pu, &p) addLabelsToDeployment(deploy, pu, &p) + deploy.ObjectMeta.Labels[containerServiceKey] = containerServiceValue + deploy.Spec.Selector.MatchLabels[containerServiceKey] = containerServiceValue + deploy.Spec.Template.ObjectMeta.Labels[containerServiceKey] = containerServiceValue if existingPort == nil || con.Ports == nil { con.Ports = append(con.Ports, corev1.ContainerPort{Name: portType, ContainerPort: portNum, Protocol: corev1.ProtocolTCP}) From b221ceaa8cb4ecdb0fd04f3a0688fcc7d2224d3a Mon Sep 17 00:00:00 2001 From: glindsell Date: Tue, 21 Jul 2020 11:07:59 +0100 Subject: [PATCH 3/8] Revert labels to bools Signed-off-by: glindsell --- .../v1/seldondeployment_types.go | 3 - operator/controllers/labels.go | 54 ++++----- operator/controllers/labels_test.go | 108 ++++++++++-------- 3 files changed, 87 insertions(+), 78 deletions(-) diff --git a/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go b/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go index 3079a63b3b..c94f629d5a 100644 --- a/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go +++ b/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go @@ -46,9 +46,6 @@ const ( Label_managed_by = "app.kubernetes.io/managed-by" Label_value_seldon = "seldon-core" - Label_component = "component" - Label_endpoint = "endpoint" - PODINFO_VOLUME_NAME = "seldon-podinfo" OLD_PODINFO_VOLUME_NAME = "podinfo" PODINFO_VOLUME_PATH = "/etc/podinfo" diff --git a/operator/controllers/labels.go b/operator/controllers/labels.go index 26955a1e28..ce3e18db3f 100644 --- a/operator/controllers/labels.go +++ b/operator/controllers/labels.go @@ -10,28 +10,28 @@ func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUni if pu.Type != nil { switch *pu.Type { case machinelearningv1.ROUTER: - svc.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_router + svc.Labels[machinelearningv1.Label_router] = "true" case machinelearningv1.COMBINER: - svc.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_combiner + svc.Labels[machinelearningv1.Label_combiner] = "true" case machinelearningv1.MODEL: - svc.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_model + svc.Labels[machinelearningv1.Label_model] = "true" case machinelearningv1.TRANSFORMER: - svc.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_transformer + svc.Labels[machinelearningv1.Label_transformer] = "true" case machinelearningv1.OUTPUT_TRANSFORMER: - svc.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_output_transformer + svc.Labels[machinelearningv1.Label_output_transformer] = "true" } } if p.Shadow != true && (p.Traffic >= 50 || p.Traffic == 0) { - svc.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_default + svc.Labels[machinelearningv1.Label_default] = "true" } if p.Shadow == true { - svc.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_shadow + svc.Labels[machinelearningv1.Label_shadow] = "true" } if p.Traffic < 50 && p.Traffic > 0 { - svc.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_canary + svc.Labels[machinelearningv1.Label_canary] = "true" } if !isEmptyExplainer(p.Explainer) { - svc.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_explainer + svc.Labels[machinelearningv1.Label_explainer] = "true" } svc.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon } @@ -40,37 +40,37 @@ func addLabelsToDeployment(deploy *appsv1.Deployment, pu *machinelearningv1.Pred if pu.Type != nil { switch *pu.Type { case machinelearningv1.ROUTER: - deploy.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_router - deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_router + deploy.Labels[machinelearningv1.Label_router] = "true" + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_router] = "true" case machinelearningv1.COMBINER: - deploy.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_combiner - deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_combiner + deploy.Labels[machinelearningv1.Label_combiner] = "true" + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_combiner] = "true" case machinelearningv1.MODEL: - deploy.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_model - deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_model + deploy.Labels[machinelearningv1.Label_model] = "true" + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_model] = "true" case machinelearningv1.TRANSFORMER: - deploy.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_transformer - deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_transformer + deploy.Labels[machinelearningv1.Label_transformer] = "true" + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_transformer] = "true" case machinelearningv1.OUTPUT_TRANSFORMER: - deploy.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_output_transformer - deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_component] = machinelearningv1.Label_output_transformer + deploy.Labels[machinelearningv1.Label_output_transformer] = "true" + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_output_transformer] = "true" } } if p.Shadow != true && (p.Traffic >= 50 || p.Traffic == 0) { - deploy.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_default - deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_default + deploy.Labels[machinelearningv1.Label_default] = "true" + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_default] = "true" } if p.Shadow == true { - deploy.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_shadow - deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_shadow + deploy.Labels[machinelearningv1.Label_shadow] = "true" + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_shadow] = "true" } if p.Traffic < 50 && p.Traffic > 0 { - deploy.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_canary - deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_canary + deploy.Labels[machinelearningv1.Label_canary] = "true" + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_canary] = "true" } if !isEmptyExplainer(p.Explainer) { - deploy.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_explainer - deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_endpoint] = machinelearningv1.Label_explainer + deploy.Labels[machinelearningv1.Label_explainer] = "true" + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_explainer] = "true" } deploy.ObjectMeta.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon diff --git a/operator/controllers/labels_test.go b/operator/controllers/labels_test.go index da6d71f32c..62cf3c59c8 100644 --- a/operator/controllers/labels_test.go +++ b/operator/controllers/labels_test.go @@ -12,12 +12,12 @@ import ( var _ = Describe("addLabelsToDeployment", func() { - var d *appsv1.Deployment + var dep *appsv1.Deployment var p *machinelearningv1.PredictorSpec var pu *machinelearningv1.PredictiveUnit BeforeEach(func() { - d = &appsv1.Deployment{ + dep = &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Labels: map[string]string{}, }, @@ -37,25 +37,26 @@ var _ = Describe("addLabelsToDeployment", func() { }) DescribeTable( - "Adds correct Component label to Deployment from Predictive Unit Type", - func(puType machinelearningv1.PredictiveUnitType, resultComponent, resultEndpoint string) { + "Adds correct label to Deployment from Predictive Unit Type", + func(puType machinelearningv1.PredictiveUnitType, result string) { pu.Type = &puType - addLabelsToDeployment(d, pu, p) + addLabelsToDeployment(dep, pu, p) - Expect(d.Labels[machinelearningv1.Label_component]).To(Equal(resultComponent)) - Expect(d.Labels[machinelearningv1.Label_endpoint]).To(Equal(resultEndpoint)) - Expect(d.Labels[machinelearningv1.Label_managed_by]).To(Equal(machinelearningv1.Label_value_seldon)) + Expect(dep.Labels[result]).To(Equal("true")) + Expect(dep.Spec.Template.ObjectMeta.Labels[result]).To(Equal("true")) + Expect(dep.Labels[machinelearningv1.Label_managed_by]).To(Equal(machinelearningv1.Label_value_seldon)) + Expect(dep.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_managed_by]).To(Equal(machinelearningv1.Label_value_seldon)) }, - Entry("router", machinelearningv1.ROUTER, machinelearningv1.Label_router, machinelearningv1.Label_default), - Entry("combiner", machinelearningv1.COMBINER, machinelearningv1.Label_combiner, machinelearningv1.Label_default), - Entry("model", machinelearningv1.MODEL, machinelearningv1.Label_model, machinelearningv1.Label_default), - Entry("transformer", machinelearningv1.TRANSFORMER, machinelearningv1.Label_transformer, machinelearningv1.Label_default), - Entry("output transformer", machinelearningv1.OUTPUT_TRANSFORMER, machinelearningv1.Label_output_transformer, machinelearningv1.Label_default), + Entry("router", machinelearningv1.ROUTER, machinelearningv1.Label_router), + Entry("combiner", machinelearningv1.COMBINER, machinelearningv1.Label_combiner), + Entry("model", machinelearningv1.MODEL, machinelearningv1.Label_model), + Entry("transformer", machinelearningv1.TRANSFORMER, machinelearningv1.Label_transformer), + Entry("output transformer", machinelearningv1.OUTPUT_TRANSFORMER, machinelearningv1.Label_output_transformer), ) DescribeTable( - "Adds correct Endpoint label to Deployment from Predictor Spec", - func(shadow bool, explainer *machinelearningv1.Explainer, traffic int, resultComponent, resultEndpoint string) { + "Adds correct label to Deployment from Predictor Spec", + func(shadow bool, explainer *machinelearningv1.Explainer, traffic int, result string, missing []string) { p.Shadow = shadow p.Explainer = explainer p.Traffic = int32(traffic) @@ -65,62 +66,66 @@ var _ = Describe("addLabelsToDeployment", func() { puType := machinelearningv1.MODEL pu.Type = &puType - addLabelsToDeployment(d, pu, p) + addLabelsToDeployment(dep, pu, p) - Expect(d.Labels[machinelearningv1.Label_component]).To(Equal(resultComponent)) - Expect(d.Labels[machinelearningv1.Label_endpoint]).To(Equal(resultEndpoint)) - - // Check template labels for pod - Expect(d.Spec.Template.Labels[machinelearningv1.Label_component]).To(Equal(resultComponent)) - Expect(d.Spec.Template.Labels[machinelearningv1.Label_endpoint]).To(Equal(resultEndpoint)) + Expect(dep.Labels[result]).To(Equal("true")) + for _, m := range missing { + Expect(dep.Labels).ToNot(HaveKey(m)) + } }, Entry( "default", false, nil, 0, - machinelearningv1.Label_model, machinelearningv1.Label_default, + []string{machinelearningv1.Label_explainer}, ), Entry( "default with 50%% traffic", false, nil, 50, - machinelearningv1.Label_model, machinelearningv1.Label_default, + []string{machinelearningv1.Label_explainer}, ), Entry( "default with 75%% traffic", false, nil, 50, - machinelearningv1.Label_model, machinelearningv1.Label_default, + []string{machinelearningv1.Label_explainer}, ), Entry( "default with empty explainer", false, &machinelearningv1.Explainer{}, 0, - machinelearningv1.Label_model, machinelearningv1.Label_default, + []string{machinelearningv1.Label_explainer}, ), Entry( "shadow", true, nil, 0, - machinelearningv1.Label_model, machinelearningv1.Label_shadow, + []string{ + machinelearningv1.Label_explainer, + machinelearningv1.Label_default, + }, ), Entry( "canary", false, nil, 48, - machinelearningv1.Label_model, machinelearningv1.Label_canary, + []string{ + machinelearningv1.Label_explainer, + machinelearningv1.Label_default, + }, ), Entry( "explainer", @@ -129,8 +134,8 @@ var _ = Describe("addLabelsToDeployment", func() { Type: machinelearningv1.AlibiAnchorsImageExplainer, }, 0, - machinelearningv1.Label_model, machinelearningv1.Label_explainer, + []string{}, ), ) }) @@ -152,25 +157,24 @@ var _ = Describe("addLabelsToService", func() { }) DescribeTable( - "Adds correct Component label to Service from Predictive Unit Type", - func(puType machinelearningv1.PredictiveUnitType, resultComponent, resultEndpoint string) { + "Adds correct label to Service from Predictive Unit Type", + func(puType machinelearningv1.PredictiveUnitType, result string) { pu.Type = &puType addLabelsToService(svc, pu, p) - Expect(svc.Labels[machinelearningv1.Label_component]).To(Equal(resultComponent)) - Expect(svc.Labels[machinelearningv1.Label_endpoint]).To(Equal(resultEndpoint)) + Expect(svc.Labels[result]).To(Equal("true")) Expect(svc.Labels[machinelearningv1.Label_managed_by]).To(Equal(machinelearningv1.Label_value_seldon)) }, - Entry("router", machinelearningv1.ROUTER, machinelearningv1.Label_router, machinelearningv1.Label_default), - Entry("combiner", machinelearningv1.COMBINER, machinelearningv1.Label_combiner, machinelearningv1.Label_default), - Entry("model", machinelearningv1.MODEL, machinelearningv1.Label_model, machinelearningv1.Label_default), - Entry("transformer", machinelearningv1.TRANSFORMER, machinelearningv1.Label_transformer, machinelearningv1.Label_default), - Entry("output transformer", machinelearningv1.OUTPUT_TRANSFORMER, machinelearningv1.Label_output_transformer, machinelearningv1.Label_default), + Entry("router", machinelearningv1.ROUTER, machinelearningv1.Label_router), + Entry("combiner", machinelearningv1.COMBINER, machinelearningv1.Label_combiner), + Entry("model", machinelearningv1.MODEL, machinelearningv1.Label_model), + Entry("transformer", machinelearningv1.TRANSFORMER, machinelearningv1.Label_transformer), + Entry("output transformer", machinelearningv1.OUTPUT_TRANSFORMER, machinelearningv1.Label_output_transformer), ) DescribeTable( "Adds correct label to Service from Predictor Spec", - func(shadow bool, explainer *machinelearningv1.Explainer, traffic int, resultComponent, resultEndpoint string) { + func(shadow bool, explainer *machinelearningv1.Explainer, traffic int, result string, missing []string) { p.Shadow = shadow p.Explainer = explainer p.Traffic = int32(traffic) @@ -182,56 +186,64 @@ var _ = Describe("addLabelsToService", func() { addLabelsToService(svc, pu, p) - Expect(svc.Labels[machinelearningv1.Label_component]).To(Equal(resultComponent)) - Expect(svc.Labels[machinelearningv1.Label_endpoint]).To(Equal(resultEndpoint)) + Expect(svc.Labels[result]).To(Equal("true")) + for _, m := range missing { + Expect(svc.Labels).ToNot(HaveKey(m)) + } }, Entry( "default", false, nil, 0, - machinelearningv1.Label_model, machinelearningv1.Label_default, + []string{machinelearningv1.Label_explainer}, ), Entry( "default with 50%% traffic", false, nil, 50, - machinelearningv1.Label_model, machinelearningv1.Label_default, + []string{machinelearningv1.Label_explainer}, ), Entry( "default with 75%% traffic", false, nil, 50, - machinelearningv1.Label_model, machinelearningv1.Label_default, + []string{machinelearningv1.Label_explainer}, ), Entry( "default with empty explainer", false, &machinelearningv1.Explainer{}, 0, - machinelearningv1.Label_model, machinelearningv1.Label_default, + []string{machinelearningv1.Label_explainer}, ), Entry( "shadow", true, nil, 0, - machinelearningv1.Label_model, machinelearningv1.Label_shadow, + []string{ + machinelearningv1.Label_explainer, + machinelearningv1.Label_default, + }, ), Entry( "canary", false, nil, 48, - machinelearningv1.Label_model, machinelearningv1.Label_canary, + []string{ + machinelearningv1.Label_explainer, + machinelearningv1.Label_default, + }, ), Entry( "explainer", @@ -240,8 +252,8 @@ var _ = Describe("addLabelsToService", func() { Type: machinelearningv1.AlibiAnchorsImageExplainer, }, 0, - machinelearningv1.Label_model, machinelearningv1.Label_explainer, + []string{}, ), ) }) From 676471b5f70d26fc5a4b3abb03bcee87d0135bfd Mon Sep 17 00:00:00 2001 From: glindsell Date: Wed, 22 Jul 2020 15:03:17 +0100 Subject: [PATCH 4/8] Add labels when creating components Signed-off-by: glindsell --- operator/controllers/labels.go | 4 +-- .../seldondeployment_controller.go | 27 +++++++++---------- .../seldondeployment_controller_test.go | 4 +-- .../controllers/seldondeployment_engine.go | 3 +-- .../seldondeployment_explainers.go | 9 ++++--- .../seldondeployment_prepackaged_servers.go | 2 +- 6 files changed, 24 insertions(+), 25 deletions(-) diff --git a/operator/controllers/labels.go b/operator/controllers/labels.go index ce3e18db3f..3aef2b2ab0 100644 --- a/operator/controllers/labels.go +++ b/operator/controllers/labels.go @@ -7,7 +7,7 @@ import ( ) func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec) { - if pu.Type != nil { + if pu != nil && pu.Type != nil { switch *pu.Type { case machinelearningv1.ROUTER: svc.Labels[machinelearningv1.Label_router] = "true" @@ -37,7 +37,7 @@ func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUni } func addLabelsToDeployment(deploy *appsv1.Deployment, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec) { - if pu.Type != nil { + if pu != nil && pu.Type != nil { switch *pu.Type { case machinelearningv1.ROUTER: deploy.Labels[machinelearningv1.Label_router] = "true" diff --git a/operator/controllers/seldondeployment_controller.go b/operator/controllers/seldondeployment_controller.go index 2b07d4b63c..ff57eab6f3 100644 --- a/operator/controllers/seldondeployment_controller.go +++ b/operator/controllers/seldondeployment_controller.go @@ -393,14 +393,13 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S for i := 0; i < len(mlDep.Spec.Predictors); i++ { p := mlDep.Spec.Predictors[i] - pu := machinelearningv1.GetPredictiveUnit(&p.Graph, p.ComponentSpecs[0].Spec.Containers[0].Name) noEngine := strings.ToLower(p.Annotations[machinelearningv1.ANNOTATION_NO_ENGINE]) == "true" pSvcName := machinelearningv1.GetPredictorKey(mlDep, &p) log.Info("pSvcName", "val", pSvcName) // Add engine deployment if separate hasSeparateEnginePod := strings.ToLower(mlDep.Spec.Annotations[machinelearningv1.ANNOTATION_SEPARATE_ENGINE]) == "true" if hasSeparateEnginePod && !noEngine { - deploy, err := createEngineDeployment(mlDep, pu, &p, pSvcName, engine_http_port, engine_grpc_port) + deploy, err := createEngineDeployment(mlDep, &p, pSvcName, engine_http_port, engine_grpc_port) if err != nil { return nil, err } @@ -423,7 +422,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S if i == 0 && j == 0 { c.defaultDeploymentName = depName } - deploy := createDeploymentWithoutEngine(depName, seldonId, cSpec, pu, &p, mlDep, securityContext) + deploy := createDeploymentWithoutEngine(depName, seldonId, cSpec, &p, mlDep, securityContext) // Add HPA if needed if cSpec.HpaSpec != nil { c.hpas = append(c.hpas, createHpa(cSpec, depName, seldonId, namespace)) @@ -443,6 +442,8 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S // get the container on the created deployment, as createDeploymentWithoutEngine will have created as a copy of the spec in the manifest and added defaults to it // we need the reference as we may have to modify the container when creating the Service (e.g. to add probes) con = utils.GetContainerForDeployment(deploy, cSpec.Spec.Containers[k].Name) + pu := machinelearningv1.GetPredictiveUnit(&p.Graph, con.Name) + addLabelsToDeployment(deploy, pu, &p) // engine will later get a special predictor service as it is entrypoint for graph // and no need to expose tfserving container as it's accessed via proxy @@ -451,6 +452,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S // service for hitting a model directly, not via engine - also adds ports to container if needed svc := createContainerService(deploy, p, mlDep, con, c, seldonId) if svc != nil { + addLabelsToService(svc, pu, &p) c.services = append(c.services, svc) } else { // a user-supplied container may not be a pu so we may not create service for that @@ -465,14 +467,14 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S port := int(svc.Spec.Ports[0].Port) - pu := machinelearningv1.GetPredictiveUnit(&p.Graph, p.ComponentSpecs[0].Spec.Containers[0].Name) if svc.Spec.Ports[0].Name == "grpc" { httpAllowed = false externalPorts[i] = httpGrpcPorts{httpPort: 0, grpcPort: port} - psvc, err := createPredictorService(pSvcName, seldonId, pu, &p, mlDep, 0, port, false, log) + psvc, err := createPredictorService(pSvcName, seldonId, &p, mlDep, 0, port, false, log) if err != nil { return nil, err } + addLabelsToService(psvc, pu, &p) c.services = append(c.services, psvc) @@ -483,10 +485,11 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S } else { externalPorts[i] = httpGrpcPorts{httpPort: port, grpcPort: 0} grpcAllowed = false - psvc, err := createPredictorService(pSvcName, seldonId, pu, &p, mlDep, port, 0, false, log) + psvc, err := createPredictorService(pSvcName, seldonId, &p, mlDep, port, 0, false, log) if err != nil { return nil, err } + addLabelsToService(psvc, pu, &p) c.services = append(c.services, psvc) @@ -552,7 +555,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S if grpcAllowed == false { grpcPort = 0 } - psvc, err := createPredictorService(pSvcName, seldonId, pu, &p, mlDep, httpPort, grpcPort, false, log) + psvc, err := createPredictorService(pSvcName, seldonId, &p, mlDep, httpPort, grpcPort, false, log) if err != nil { return nil, err @@ -606,7 +609,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S } //Creates Service for Predictor - exposed externally (ambassador or istio) -func createPredictorService(pSvcName string, seldonId string, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec, +func createPredictorService(pSvcName string, seldonId string, p *machinelearningv1.PredictorSpec, mlDep *machinelearningv1.SeldonDeployment, engine_http_port int, engine_grpc_port int, @@ -660,7 +663,6 @@ func createPredictorService(pSvcName string, seldonId string, pu *machinelearnin log.Info("Creating Headless SVC") psvc.Spec.ClusterIP = "None" } - addLabelsToService(psvc, pu, p) return psvc, err } @@ -733,8 +735,6 @@ func createContainerService(deploy *appsv1.Deployment, SessionAffinity: corev1.ServiceAffinityNone, }, } - addLabelsToService(svc, pu, &p) - addLabelsToDeployment(deploy, pu, &p) deploy.ObjectMeta.Labels[containerServiceKey] = containerServiceValue deploy.Spec.Selector.MatchLabels[containerServiceKey] = containerServiceValue deploy.Spec.Template.ObjectMeta.Labels[containerServiceKey] = containerServiceValue @@ -794,7 +794,7 @@ func createContainerService(deploy *appsv1.Deployment, return svc } -func createDeploymentWithoutEngine(depName string, seldonId string, seldonPodSpec *machinelearningv1.SeldonPodSpec, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec, mlDep *machinelearningv1.SeldonDeployment, podSecurityContext *corev1.PodSecurityContext) *appsv1.Deployment { +func createDeploymentWithoutEngine(depName string, seldonId string, seldonPodSpec *machinelearningv1.SeldonPodSpec, p *machinelearningv1.PredictorSpec, mlDep *machinelearningv1.SeldonDeployment, podSecurityContext *corev1.PodSecurityContext) *appsv1.Deployment { deploy := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: depName, @@ -897,9 +897,6 @@ func createDeploymentWithoutEngine(depName string, seldonId string, seldonPodSpe DownwardAPI: &corev1.DownwardAPIVolumeSource{Items: []corev1.DownwardAPIVolumeFile{ {Path: "annotations", FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.annotations", APIVersion: "v1"}}}, DefaultMode: &defaultMode}}}) } - - addLabelsToDeployment(deploy, pu, p) - return deploy } diff --git a/operator/controllers/seldondeployment_controller_test.go b/operator/controllers/seldondeployment_controller_test.go index 3e030f8c41..f042899634 100644 --- a/operator/controllers/seldondeployment_controller_test.go +++ b/operator/controllers/seldondeployment_controller_test.go @@ -870,7 +870,7 @@ func TestCreateDeploymentWithLabelsAndAnnotations(t *testing.T) { }, } - dep := createDeploymentWithoutEngine(depName, "a", instance.Spec.Predictors[0].ComponentSpecs[0], &instance.Spec.Predictors[0].Graph, &instance.Spec.Predictors[0], instance, nil) + dep := createDeploymentWithoutEngine(depName, "a", instance.Spec.Predictors[0].ComponentSpecs[0], &instance.Spec.Predictors[0], instance, nil) g.Expect(dep.Labels[labelKey1]).To(Equal(labelValue1)) g.Expect(dep.Labels[labelKey2]).To(Equal(labelValue2)) g.Expect(dep.Spec.Template.ObjectMeta.Labels[labelKey1]).To(Equal(labelValue1)) @@ -915,5 +915,5 @@ func TestCreateDeploymentWithNoLabelsAndAnnotations(t *testing.T) { }, } - _ = createDeploymentWithoutEngine(depName, "a", instance.Spec.Predictors[0].ComponentSpecs[0], &instance.Spec.Predictors[0].Graph, &instance.Spec.Predictors[0], instance, nil) + _ = createDeploymentWithoutEngine(depName, "a", instance.Spec.Predictors[0].ComponentSpecs[0], &instance.Spec.Predictors[0], instance, nil) } diff --git a/operator/controllers/seldondeployment_engine.go b/operator/controllers/seldondeployment_engine.go index fe99c9ed5d..9db808ee0e 100644 --- a/operator/controllers/seldondeployment_engine.go +++ b/operator/controllers/seldondeployment_engine.go @@ -390,7 +390,7 @@ func createEngineContainer(mlDep *machinelearningv1.SeldonDeployment, p *machine } // Create the service orchestrator. -func createEngineDeployment(mlDep *machinelearningv1.SeldonDeployment, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec, seldonId string, engine_http_port, engine_grpc_port int) (*appsv1.Deployment, error) { +func createEngineDeployment(mlDep *machinelearningv1.SeldonDeployment, p *machinelearningv1.PredictorSpec, seldonId string, engine_http_port, engine_grpc_port int) (*appsv1.Deployment, error) { var terminationGracePeriodSecs = int64(20) var defaultMode = corev1.DownwardAPIVolumeSourceDefaultMode @@ -467,6 +467,5 @@ func createEngineDeployment(mlDep *machinelearningv1.SeldonDeployment, pu *machi deploy.ObjectMeta.Labels[k] = v deploy.Spec.Template.ObjectMeta.Labels[k] = v } - addLabelsToDeployment(deploy, pu, p) return deploy, nil } diff --git a/operator/controllers/seldondeployment_explainers.go b/operator/controllers/seldondeployment_explainers.go index 2795f2a5fc..c3dfaad122 100644 --- a/operator/controllers/seldondeployment_explainers.go +++ b/operator/controllers/seldondeployment_explainers.go @@ -82,7 +82,6 @@ func getExplainerConfigsFromMap(configMap *corev1.ConfigMap) (*ExplainerConfig, func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonDeployment, p *machinelearningv1.PredictorSpec, c *components, pSvcName string, podSecurityContect *corev1.PodSecurityContext, log logr.Logger) error { if !isEmptyExplainer(p.Explainer) { - pu := machinelearningv1.GetPredictiveUnit(&p.Graph, p.ComponentSpecs[0].Spec.Containers[0].Name) seldonId := machinelearningv1.GetSeldonDeploymentName(mlDep) @@ -94,6 +93,8 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD explainerContainer.Name = depName } + pu := machinelearningv1.GetPredictiveUnit(&p.Graph, explainerContainer.Name) + if explainerContainer.ImagePullPolicy == "" { explainerContainer.ImagePullPolicy = corev1.PullIfNotPresent } @@ -204,7 +205,7 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD Containers: []corev1.Container{explainerContainer}, }} - deploy := createDeploymentWithoutEngine(depName, seldonId, &seldonPodSpec, pu, p, mlDep, podSecurityContect) + deploy := createDeploymentWithoutEngine(depName, seldonId, &seldonPodSpec, p, mlDep, podSecurityContect) if p.Explainer.ModelUri != "" { var err error @@ -219,16 +220,18 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD // for explainer use same service name as its Deployment eSvcName := machinelearningv1.GetExplainerDeploymentName(mlDep.GetName(), p) + addLabelsToDeployment(deploy, pu, p) deploy.ObjectMeta.Labels[machinelearningv1.Label_seldon_app] = eSvcName deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_seldon_app] = eSvcName c.deployments = append(c.deployments, deploy) // Use seldondeployment name dash explainer as the external service name. This should allow canarying. - eSvc, err := createPredictorService(eSvcName, seldonId, pu, p, mlDep, httpPort, grpcPort, true, log) + eSvc, err := createPredictorService(eSvcName, seldonId, p, mlDep, httpPort, grpcPort, true, log) if err != nil { return err } + addLabelsToService(eSvc, pu, p) c.services = append(c.services, eSvc) c.serviceDetails[eSvcName] = &machinelearningv1.ServiceStatus{ SvcName: eSvcName, diff --git a/operator/controllers/seldondeployment_prepackaged_servers.go b/operator/controllers/seldondeployment_prepackaged_servers.go index d04b1f2c92..2efb8aa3d6 100644 --- a/operator/controllers/seldondeployment_prepackaged_servers.go +++ b/operator/controllers/seldondeployment_prepackaged_servers.go @@ -283,7 +283,7 @@ func (pi *PrePackedInitialiser) createStandaloneModelServers(mlDep *machinelearn // might not be a Deployment yet - if so we have to create one if deploy == nil { seldonId := machinelearningv1.GetSeldonDeploymentName(mlDep) - deploy = createDeploymentWithoutEngine(depName, seldonId, sPodSpec, pu, p, mlDep, podSecurityContext) + deploy = createDeploymentWithoutEngine(depName, seldonId, sPodSpec, p, mlDep, podSecurityContext) } // apply serviceAccountName to pod to enable EKS fine-grained IAM roles From 48bb5260d02b9a7a145f0727c80fe53f672004fd Mon Sep 17 00:00:00 2001 From: glindsell Date: Wed, 22 Jul 2020 18:07:24 +0100 Subject: [PATCH 5/8] Remove explainer label from other pu types Signed-off-by: glindsell --- operator/controllers/labels.go | 12 +++++------- operator/controllers/labels_test.go | 16 ++++++++++++---- .../controllers/seldondeployment_explainers.go | 6 +++--- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/operator/controllers/labels.go b/operator/controllers/labels.go index 3aef2b2ab0..1a39b3483a 100644 --- a/operator/controllers/labels.go +++ b/operator/controllers/labels.go @@ -20,6 +20,8 @@ func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUni case machinelearningv1.OUTPUT_TRANSFORMER: svc.Labels[machinelearningv1.Label_output_transformer] = "true" } + } else if !isEmptyExplainer(p.Explainer) { + svc.Labels[machinelearningv1.Label_explainer] = "true" } if p.Shadow != true && (p.Traffic >= 50 || p.Traffic == 0) { svc.Labels[machinelearningv1.Label_default] = "true" @@ -30,9 +32,6 @@ func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUni if p.Traffic < 50 && p.Traffic > 0 { svc.Labels[machinelearningv1.Label_canary] = "true" } - if !isEmptyExplainer(p.Explainer) { - svc.Labels[machinelearningv1.Label_explainer] = "true" - } svc.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon } @@ -55,6 +54,9 @@ func addLabelsToDeployment(deploy *appsv1.Deployment, pu *machinelearningv1.Pred deploy.Labels[machinelearningv1.Label_output_transformer] = "true" deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_output_transformer] = "true" } + } else if !isEmptyExplainer(p.Explainer) { + deploy.Labels[machinelearningv1.Label_explainer] = "true" + deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_explainer] = "true" } if p.Shadow != true && (p.Traffic >= 50 || p.Traffic == 0) { deploy.Labels[machinelearningv1.Label_default] = "true" @@ -68,10 +70,6 @@ func addLabelsToDeployment(deploy *appsv1.Deployment, pu *machinelearningv1.Pred deploy.Labels[machinelearningv1.Label_canary] = "true" deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_canary] = "true" } - if !isEmptyExplainer(p.Explainer) { - deploy.Labels[machinelearningv1.Label_explainer] = "true" - deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_explainer] = "true" - } deploy.ObjectMeta.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon } diff --git a/operator/controllers/labels_test.go b/operator/controllers/labels_test.go index 62cf3c59c8..3cc8859369 100644 --- a/operator/controllers/labels_test.go +++ b/operator/controllers/labels_test.go @@ -63,8 +63,12 @@ var _ = Describe("addLabelsToDeployment", func() { // Required until https://github.com/SeldonIO/seldon-core/pull/1600 is // merged. We should remove afterwards. - puType := machinelearningv1.MODEL - pu.Type = &puType + if explainer != nil { + pu.Type = nil + } else { + puType := machinelearningv1.MODEL + pu.Type = &puType + } addLabelsToDeployment(dep, pu, p) @@ -181,8 +185,12 @@ var _ = Describe("addLabelsToService", func() { // Required until https://github.com/SeldonIO/seldon-core/pull/1600 is // merged. We should remove afterwards. - puType := machinelearningv1.MODEL - pu.Type = &puType + if explainer != nil { + pu.Type = nil + } else { + puType := machinelearningv1.MODEL + pu.Type = &puType + } addLabelsToService(svc, pu, p) diff --git a/operator/controllers/seldondeployment_explainers.go b/operator/controllers/seldondeployment_explainers.go index c3dfaad122..eb2e3dfebb 100644 --- a/operator/controllers/seldondeployment_explainers.go +++ b/operator/controllers/seldondeployment_explainers.go @@ -93,7 +93,7 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD explainerContainer.Name = depName } - pu := machinelearningv1.GetPredictiveUnit(&p.Graph, explainerContainer.Name) + //pu := machinelearningv1.GetPredictiveUnit(&p.Graph, explainerContainer.Name) if explainerContainer.ImagePullPolicy == "" { explainerContainer.ImagePullPolicy = corev1.PullIfNotPresent @@ -220,7 +220,7 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD // for explainer use same service name as its Deployment eSvcName := machinelearningv1.GetExplainerDeploymentName(mlDep.GetName(), p) - addLabelsToDeployment(deploy, pu, p) + addLabelsToDeployment(deploy, nil, p) deploy.ObjectMeta.Labels[machinelearningv1.Label_seldon_app] = eSvcName deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_seldon_app] = eSvcName @@ -231,7 +231,7 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD if err != nil { return err } - addLabelsToService(eSvc, pu, p) + addLabelsToService(eSvc, nil, p) c.services = append(c.services, eSvc) c.serviceDetails[eSvcName] = &machinelearningv1.ServiceStatus{ SvcName: eSvcName, From b1987e267ec1e6948f8ad2fbfa99e6c277d5b3f1 Mon Sep 17 00:00:00 2001 From: glindsell Date: Wed, 22 Jul 2020 18:09:31 +0100 Subject: [PATCH 6/8] Remove comment Signed-off-by: glindsell --- operator/controllers/seldondeployment_explainers.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/operator/controllers/seldondeployment_explainers.go b/operator/controllers/seldondeployment_explainers.go index eb2e3dfebb..ba3ea8400b 100644 --- a/operator/controllers/seldondeployment_explainers.go +++ b/operator/controllers/seldondeployment_explainers.go @@ -93,8 +93,6 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD explainerContainer.Name = depName } - //pu := machinelearningv1.GetPredictiveUnit(&p.Graph, explainerContainer.Name) - if explainerContainer.ImagePullPolicy == "" { explainerContainer.ImagePullPolicy = corev1.PullIfNotPresent } From d3f15e49d01dd7c92bf7ed904d4e7f407a86b0df Mon Sep 17 00:00:00 2001 From: glindsell Date: Thu, 23 Jul 2020 14:24:25 +0100 Subject: [PATCH 7/8] Add seldon.io prefix to labels Signed-off-by: glindsell --- .../v1/seldondeployment_types.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go b/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go index c94f629d5a..71195a59fd 100644 --- a/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go +++ b/operator/apis/machinelearning.seldon.io/v1/seldondeployment_types.go @@ -34,15 +34,15 @@ const ( Label_svc_orch = "seldon-deployment-contains-svcorch" Label_app = "app" Label_fluentd = "fluentd" - Label_router = "router" - Label_combiner = "combiner" - Label_model = "model" - Label_transformer = "transformer" - Label_output_transformer = "output-transformer" - Label_default = "default" - Label_shadow = "shadow" - Label_canary = "canary" - Label_explainer = "explainer" + Label_router = "seldon.io/router" + Label_combiner = "seldon.io/combiner" + Label_model = "seldon.io/model" + Label_transformer = "seldon.io/transformer" + Label_output_transformer = "seldon.io/output-transformer" + Label_default = "seldon.io/default" + Label_shadow = "seldon.io/shadow" + Label_canary = "seldon.io/canary" + Label_explainer = "seldon.io/explainer" Label_managed_by = "app.kubernetes.io/managed-by" Label_value_seldon = "seldon-core" From fcc4826f0a1d146217b3eec161e1824d8b184342 Mon Sep 17 00:00:00 2001 From: glindsell Date: Fri, 24 Jul 2020 17:08:07 +0100 Subject: [PATCH 8/8] Return deployments and services when adding labels Signed-off-by: glindsell --- operator/controllers/labels.go | 6 ++++-- operator/controllers/seldondeployment_controller.go | 8 ++++---- operator/controllers/seldondeployment_explainers.go | 4 ++-- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/operator/controllers/labels.go b/operator/controllers/labels.go index 1a39b3483a..2b3e6cab50 100644 --- a/operator/controllers/labels.go +++ b/operator/controllers/labels.go @@ -6,7 +6,7 @@ import ( corev1 "k8s.io/api/core/v1" ) -func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec) { +func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec) *corev1.Service { if pu != nil && pu.Type != nil { switch *pu.Type { case machinelearningv1.ROUTER: @@ -33,9 +33,10 @@ func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUni svc.Labels[machinelearningv1.Label_canary] = "true" } svc.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon + return svc } -func addLabelsToDeployment(deploy *appsv1.Deployment, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec) { +func addLabelsToDeployment(deploy *appsv1.Deployment, pu *machinelearningv1.PredictiveUnit, p *machinelearningv1.PredictorSpec) *appsv1.Deployment { if pu != nil && pu.Type != nil { switch *pu.Type { case machinelearningv1.ROUTER: @@ -72,4 +73,5 @@ func addLabelsToDeployment(deploy *appsv1.Deployment, pu *machinelearningv1.Pred } deploy.ObjectMeta.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_managed_by] = machinelearningv1.Label_value_seldon + return deploy } diff --git a/operator/controllers/seldondeployment_controller.go b/operator/controllers/seldondeployment_controller.go index ff57eab6f3..900b5b2b48 100644 --- a/operator/controllers/seldondeployment_controller.go +++ b/operator/controllers/seldondeployment_controller.go @@ -443,7 +443,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S // we need the reference as we may have to modify the container when creating the Service (e.g. to add probes) con = utils.GetContainerForDeployment(deploy, cSpec.Spec.Containers[k].Name) pu := machinelearningv1.GetPredictiveUnit(&p.Graph, con.Name) - addLabelsToDeployment(deploy, pu, &p) + deploy = addLabelsToDeployment(deploy, pu, &p) // engine will later get a special predictor service as it is entrypoint for graph // and no need to expose tfserving container as it's accessed via proxy @@ -452,7 +452,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S // service for hitting a model directly, not via engine - also adds ports to container if needed svc := createContainerService(deploy, p, mlDep, con, c, seldonId) if svc != nil { - addLabelsToService(svc, pu, &p) + svc = addLabelsToService(svc, pu, &p) c.services = append(c.services, svc) } else { // a user-supplied container may not be a pu so we may not create service for that @@ -474,7 +474,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S if err != nil { return nil, err } - addLabelsToService(psvc, pu, &p) + psvc = addLabelsToService(psvc, pu, &p) c.services = append(c.services, psvc) @@ -489,7 +489,7 @@ func (r *SeldonDeploymentReconciler) createComponents(mlDep *machinelearningv1.S if err != nil { return nil, err } - addLabelsToService(psvc, pu, &p) + psvc = addLabelsToService(psvc, pu, &p) c.services = append(c.services, psvc) diff --git a/operator/controllers/seldondeployment_explainers.go b/operator/controllers/seldondeployment_explainers.go index ba3ea8400b..77e77fba2e 100644 --- a/operator/controllers/seldondeployment_explainers.go +++ b/operator/controllers/seldondeployment_explainers.go @@ -218,7 +218,7 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD // for explainer use same service name as its Deployment eSvcName := machinelearningv1.GetExplainerDeploymentName(mlDep.GetName(), p) - addLabelsToDeployment(deploy, nil, p) + deploy = addLabelsToDeployment(deploy, nil, p) deploy.ObjectMeta.Labels[machinelearningv1.Label_seldon_app] = eSvcName deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_seldon_app] = eSvcName @@ -229,7 +229,7 @@ func (ei *ExplainerInitialiser) createExplainer(mlDep *machinelearningv1.SeldonD if err != nil { return err } - addLabelsToService(eSvc, nil, p) + eSvc = addLabelsToService(eSvc, nil, p) c.services = append(c.services, eSvc) c.serviceDetails[eSvcName] = &machinelearningv1.ServiceStatus{ SvcName: eSvcName,