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

feat(controller): allow setting the Experiment's service name; don't hardcode ports #2357

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
952d043
Copy ports from replicaset rather than hardcoding them
alexef Oct 21, 2022
69e9061
Allow user to set the experiment service name.
alexef Oct 21, 2022
d6ae5ad
Run codegen
alexef Oct 21, 2022
666d888
Run codegen again, service name is optional
alexef Oct 21, 2022
431f894
Fix conversion and tests
alexef Oct 21, 2022
49b01f1
Fix codegen
alexef Oct 21, 2022
21adb4a
Fix codegen again
alexef Oct 21, 2022
f534f9c
white space
alexef Oct 21, 2022
2b22947
Extend from corev1.ServiceSpec
alexef Oct 22, 2022
0f28e2b
make lint
alexef Oct 22, 2022
6525b3f
Merge branch 'master' into experiment-service-name
alexef Oct 23, 2022
d805441
infer ports from ReplicaSet. this should fix e2e tests
alexef Oct 23, 2022
5b66607
Merge remote-tracking branch 'origin/experiment-service-name' into ex…
alexef Oct 23, 2022
303b29e
fix e2e
alexef Oct 23, 2022
d1371cd
use json inline so that crd reflects ports
alexef Oct 23, 2022
e1247e1
Add codegen openapi_generated.go
alexef Oct 23, 2022
b8c4157
fix codegen, protobuf is needed
alexef Oct 23, 2022
8c671cc
Merge branch 'master' into experiment-service-name
alexef Oct 24, 2022
de6b858
Add unit test
alexef Oct 24, 2022
bc379f7
Add unit test to new not tested code - copy pods from RS
alexef Oct 24, 2022
9437f6d
Merge branch 'master' into experiment-service-name
alexef Oct 31, 2022
9a179d4
Merge branch 'master' into experiment-service-name
alexef Nov 1, 2022
c32cbe7
Address PR comment: set entire spec
alexef Nov 1, 2022
0f5ed7a
docs: update experiment and rollout spec
alexef Nov 1, 2022
8f46d8c
Merge branch 'master' into experiment-service-name
alexef Nov 1, 2022
95b48ca
Merge branch 'master' into experiment-service-name
alexef Nov 3, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions docs/features/experiment.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ The Experiment CRD allows users to have ephemeral runs of one or more ReplicaSet
running ephemeral ReplicaSets, the Experiment CRD can launch AnalysisRuns alongside the ReplicaSets.
Generally, those AnalysisRun is used to confirm that new ReplicaSets are running as expected.

A Service routing traffic to the Experiment ReplicaSet is generated if either:
- the experiment template is weighted;
- the experiment template explicitly defines the `service` property.

## Use cases of Experiments

- A user wants to run two versions of an application for a specific duration to enable Kayenta-style
Expand Down Expand Up @@ -67,6 +71,18 @@ spec:
- name: http
containerPort: 8080
protocol: TCP
# Generate a Service object pointing to this variation
service:
name: experiment-purple
# Control the service specification (selector, ports etc)
ports:
- name: http
targetPort: 8080
port: 80
protocol: TCP
selector:
app: canary-demo
color: purple
- name: orange
replicas: 1
minReadySeconds: 10
Expand Down Expand Up @@ -243,5 +259,10 @@ to `experiment-baseline`, leaving the remaining 90% of traffic to the old stack.

!!! note
When a weighted experiment step with traffic routing is used, a
service is auto-created for each experiment template. The traffic routers use
this service to send traffic to the experiment pods.
Service is auto-created for each experiment template. The traffic routers use
this service to send traffic to the experiment pods.

By default, the generated Service has the name of the ReplicaSet and inherits
ports and selector from the specRef definition. These properties can be overriden,
using the `experiment.templates[].service` specification
(see [Experiment Spec](#experiment-spec)).
134 changes: 134 additions & 0 deletions docs/features/kustomize/rollout_cr_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -12969,6 +12969,140 @@
"type": "object"
},
"service": {
"properties": {
"allocateLoadBalancerNodePorts": {
"type": "boolean"
},
"clusterIP": {
"type": "string"
},
"clusterIPs": {
"items": {
"type": "string"
},
"type": "array",
"x-kubernetes-list-type": "atomic"
},
"externalIPs": {
"items": {
"type": "string"
},
"type": "array"
},
"externalName": {
"type": "string"
},
"externalTrafficPolicy": {
"type": "string"
},
"healthCheckNodePort": {
"format": "int32",
"type": "integer"
},
"internalTrafficPolicy": {
"type": "string"
},
"ipFamilies": {
"items": {
"type": "string"
},
"type": "array",
"x-kubernetes-list-type": "atomic"
},
"ipFamilyPolicy": {
"type": "string"
},
"loadBalancerClass": {
"type": "string"
},
"loadBalancerIP": {
"type": "string"
},
"loadBalancerSourceRanges": {
"items": {
"type": "string"
},
"type": "array"
},
"name": {
"type": "string"
},
"ports": {
"items": {
"properties": {
"appProtocol": {
"type": "string"
},
"name": {
"type": "string"
},
"nodePort": {
"format": "int32",
"type": "integer"
},
"port": {
"format": "int32",
"type": "integer"
},
"protocol": {
"default": "TCP",
"type": "string"
},
"targetPort": {
"anyOf": [
{
"type": "integer"
},
{
"type": "string"
}
],
"x-kubernetes-int-or-string": true
}
},
"required": [
"port"
],
"type": "object"
},
"type": "array",
"x-kubernetes-list-map-keys": [
"port",
"protocol"
],
"x-kubernetes-list-type": "map"
},
"publishNotReadyAddresses": {
"type": "boolean"
},
"selector": {
"additionalProperties": {
"type": "string"
},
"type": "object",
"x-kubernetes-map-type": "atomic"
},
"sessionAffinity": {
"type": "string"
},
"sessionAffinityConfig": {
"properties": {
"clientIP": {
"properties": {
"timeoutSeconds": {
"format": "int32",
"type": "integer"
}
},
"type": "object"
}
},
"type": "object"
},
"type": {
"type": "string"
}
},
"type": "object"
},
"template": {
Expand Down
13 changes: 13 additions & 0 deletions docs/features/specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,21 @@ spec:
templates:
- name: baseline
specRef: stable
# optional, generate a Service pointing to this version
# by default, the service is named the same as the ReplicaSet
# but name can be explicitly set below
service:
name: experiment-baseline
# optional, control the service specification (selector, ports etc)
ports:
- name: http
targetPort: 8080
port: 80
protocol: TCP
- name: canary
specRef: canary
# optional, set the weight of traffic routed to this version
weight: 10
analyses:
- name : mann-whitney
templateName: mann-whitney
Expand Down
26 changes: 24 additions & 2 deletions experiments/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
corev1 "k8s.io/api/core/v1"
k8serrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/client-go/kubernetes"
appslisters "k8s.io/client-go/listers/apps/v1"
v1 "k8s.io/client-go/listers/core/v1"
Expand Down Expand Up @@ -285,8 +286,29 @@ func (ec *experimentContext) createTemplateService(template *v1alpha1.TemplateSp
// Create service with has same name, podTemplateHash, and labels as RS
podTemplateHash := rs.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]
svc := ec.templateServices[template.Name]
if svc == nil || svc.Name != rs.Name {
newService, err := ec.CreateService(rs.Name, *template, rs.Labels)
templateServiceName := template.Service.Name
if templateServiceName == "" {
templateServiceName = rs.Name
}
if len(template.Service.Selector) == 0 {
template.Service.Selector = rs.Labels
}
if len(template.Service.Ports) == 0 {
var ports []corev1.ServicePort
for _, ctr := range rs.Spec.Template.Spec.Containers {
for _, port := range ctr.Ports {
servicePort := corev1.ServicePort{
Protocol: port.Protocol,
Port: port.ContainerPort,
TargetPort: intstr.FromInt(int(port.ContainerPort)),
}
ports = append(ports, servicePort)
}
}
template.Service.Ports = ports
}
if svc == nil || svc.Name != templateServiceName {
newService, err := ec.CreateService(templateServiceName, *template)
if err != nil {
templateStatus.Status = v1alpha1.TemplateStatusError
templateStatus.Message = fmt.Sprintf("Failed to create Service for template '%s': %v", template.Name, err)
Expand Down
23 changes: 23 additions & 0 deletions experiments/experiment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -496,3 +496,26 @@ func TestDeleteServiceIfServiceFieldNil(t *testing.T) {
assert.Equal(t, "", exStatus.TemplateStatuses[0].ServiceName)
assert.Nil(t, exCtx.templateServices["bar"])
}

func TestServiceInheritPortsFromRS(t *testing.T) {
templates := generateTemplates("bar")
templates[0].Service = &v1alpha1.TemplateService{Name: "foobar"}
templates[0].Template.Spec.Containers[0].Ports = []corev1.ContainerPort{
{
Name: "testport",
ContainerPort: 80,
Protocol: "TCP",
},
}
ex := newExperiment("foo", templates, "")

exCtx := newTestContext(ex)
rs := templateToRS(ex, templates[0], 0)
exCtx.templateRSs["bar"] = rs

exCtx.reconcile()

assert.NotNil(t, exCtx.templateServices["bar"])
assert.Equal(t, exCtx.templateServices["bar"].Name, "foobar")
assert.Equal(t, exCtx.templateServices["bar"].Spec.Ports[0].Port, int32(80))
}
16 changes: 4 additions & 12 deletions experiments/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
"k8s.io/apimachinery/pkg/util/intstr"
)

var experimentKind = v1alpha1.SchemeGroupVersion.WithKind("Experiment")
Expand Down Expand Up @@ -59,9 +58,9 @@ func GetServiceForExperiment(experiment *v1alpha1.Experiment, svc *corev1.Servic
return nil
}

func (ec *experimentContext) CreateService(serviceName string, template v1alpha1.TemplateSpec, selector map[string]string) (*corev1.Service, error) {
func (ec *experimentContext) CreateService(serviceName string, template v1alpha1.TemplateSpec) (*corev1.Service, error) {
ctx := context.TODO()
serviceAnnotations := newServiceSetAnnotations(ec.ex.Name, template.Name)
serviceAnnotations := newServiceAnnotations(ec.ex.Name, template.Name)
newService := &corev1.Service{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -72,14 +71,7 @@ func (ec *experimentContext) CreateService(serviceName string, template v1alpha1
},
Annotations: serviceAnnotations,
},
Spec: corev1.ServiceSpec{
Selector: selector,
Ports: []corev1.ServicePort{{
Protocol: "TCP",
Port: int32(80),
TargetPort: intstr.FromInt(8080),
}},
},
Spec: template.Service.ServiceSpec,
}

service, err := ec.kubeclientset.CoreV1().Services(ec.ex.Namespace).Create(ctx, newService, metav1.CreateOptions{})
Expand Down Expand Up @@ -112,7 +104,7 @@ func (ec *experimentContext) deleteService(service corev1.Service) error {
return nil
}

func newServiceSetAnnotations(experimentName, templateName string) map[string]string {
func newServiceAnnotations(experimentName, templateName string) map[string]string {
return map[string]string{
v1alpha1.ExperimentNameAnnotationKey: experimentName,
v1alpha1.ExperimentTemplateNameAnnotationKey: templateName,
Expand Down
Loading