Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add labels to deployments and improve label coverage of resources #2130

Merged
merged 11 commits into from
Jul 30, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! Namespacing the labels under seldon.io/* seems like the right thing to do 👍

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"

Expand Down
50 changes: 41 additions & 9 deletions operator/controllers/labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import (
corev1 "k8s.io/api/core/v1"
)

func addLabelsToService(svc *corev1.Service, pu *machinelearningv1.PredictiveUnit, p machinelearningv1.PredictorSpec) {
if pu.Type != nil {
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:
svc.Labels[machinelearningv1.Label_router] = "true"
Expand All @@ -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"
Expand All @@ -30,16 +32,46 @@ 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
return svc
}

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) *appsv1.Deployment {
if pu != nil && pu.Type != nil {
switch *pu.Type {
case machinelearningv1.ROUTER:
deploy.Labels[machinelearningv1.Label_router] = "true"
deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_router] = "true"
case machinelearningv1.COMBINER:
deploy.Labels[machinelearningv1.Label_combiner] = "true"
deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_combiner] = "true"
case machinelearningv1.MODEL:
deploy.Labels[machinelearningv1.Label_model] = "true"
deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_model] = "true"
case machinelearningv1.TRANSFORMER:
deploy.Labels[machinelearningv1.Label_transformer] = "true"
deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_transformer] = "true"
case machinelearningv1.OUTPUT_TRANSFORMER:
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"
deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_default] = "true"
}
if p.Shadow == true {
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_canary] = "true"
deploy.Spec.Template.ObjectMeta.Labels[machinelearningv1.Label_canary] = "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
return deploy
}
165 changes: 137 additions & 28 deletions operator/controllers/labels_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package controllers

import (
"testing"

. "github.com/onsi/ginkgo"
. "github.com/onsi/ginkgo/extensions/table"
. "github.com/onsi/gomega"
Expand All @@ -12,37 +10,144 @@ 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 dep *appsv1.Deployment
var p *machinelearningv1.PredictorSpec
var pu *machinelearningv1.PredictiveUnit

BeforeEach(func() {
dep = &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 label to Deployment from Predictive Unit Type",
func(puType machinelearningv1.PredictiveUnitType, result string) {
pu.Type = &puType
addLabelsToDeployment(dep, pu, p)

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),
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 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)

// Required until https://github.com/SeldonIO/seldon-core/pull/1600 is
// merged. We should remove afterwards.
if explainer != nil {
pu.Type = nil
} else {
puType := machinelearningv1.MODEL
pu.Type = &puType
}

addLabelsToDeployment(dep, pu, p)

Expect(dep.Labels[result]).To(Equal("true"))
for _, m := range missing {
Expect(dep.Labels).ToNot(HaveKey(m))
}
},
Entry(
"default",
false,
nil,
0,
machinelearningv1.Label_default,
[]string{machinelearningv1.Label_explainer},
),
Entry(
"default with 50%% traffic",
false,
nil,
50,
machinelearningv1.Label_default,
[]string{machinelearningv1.Label_explainer},
),
Entry(
"default with 75%% traffic",
false,
nil,
50,
machinelearningv1.Label_default,
[]string{machinelearningv1.Label_explainer},
),
Entry(
"default with empty explainer",
false,
&machinelearningv1.Explainer{},
0,
machinelearningv1.Label_default,
[]string{machinelearningv1.Label_explainer},
),
Entry(
"shadow",
true,
nil,
0,
machinelearningv1.Label_shadow,
[]string{
machinelearningv1.Label_explainer,
machinelearningv1.Label_default,
},
),
Entry(
"canary",
false,
nil,
48,
machinelearningv1.Label_canary,
[]string{
machinelearningv1.Label_explainer,
machinelearningv1.Label_default,
},
),
Entry(
"explainer",
false,
&machinelearningv1.Explainer{
Type: machinelearningv1.AlibiAnchorsImageExplainer,
},
0,
machinelearningv1.Label_explainer,
[]string{},
),
)
})

var _ = Describe("addLabelsToService", func() {

var svc *corev1.Service
var p machinelearningv1.PredictorSpec
var p *machinelearningv1.PredictorSpec
var pu *machinelearningv1.PredictiveUnit

BeforeEach(func() {
Expand All @@ -51,7 +156,7 @@ var _ = Describe("addLabelsToService", func() {
Labels: map[string]string{},
},
}
p = machinelearningv1.PredictorSpec{}
p = &machinelearningv1.PredictorSpec{}
pu = &machinelearningv1.PredictiveUnit{}
})

Expand Down Expand Up @@ -80,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)

Expand Down
Loading