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: Add Service field to Rollout Experiment to allow service creation #2633

Merged
merged 21 commits into from
Mar 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
ef56a06
Add Service field to Rollout Experiment to allow service creation
daniddelrio Mar 1, 2023
38b8082
Merge branch 'master' into service-in-experiment
daniddelrio Mar 2, 2023
1615a26
Merge branch 'master' into service-in-experiment
daniddelrio Mar 3, 2023
4f6abea
Change template names
daniddelrio Mar 3, 2023
dd04f9f
Merge branch 'master' into service-in-experiment
daniddelrio Mar 6, 2023
010a3e5
Merge branch 'master' into service-in-experiment
daniddelrio Mar 9, 2023
30528d2
Add test for service without name
daniddelrio Mar 9, 2023
0e21267
Remove Service field in trafficrouting_test
daniddelrio Mar 9, 2023
c657cc2
Add e2e test for experiment with service name
daniddelrio Mar 9, 2023
ecfc87e
Update specifications and documentation about setting Service in expe…
daniddelrio Mar 9, 2023
7dcafa1
Pass Service name instead of replicaset name to CreateService func in…
daniddelrio Mar 9, 2023
98d8598
Modify unit/e2e tests for service name
daniddelrio Mar 9, 2023
2f9f3e4
Change app name in experiment e2e test
daniddelrio Mar 9, 2023
d33a874
Add to error messages
daniddelrio Mar 9, 2023
602c2d3
Merge branch 'master' into service-in-experiment
daniddelrio Mar 9, 2023
d71d730
Fix unit test for service creation
daniddelrio Mar 9, 2023
a629acf
Merge branch 'master' into service-in-experiment
daniddelrio Mar 13, 2023
31fca4d
Change service.Name call in createService
daniddelrio Mar 13, 2023
da4c9d0
Merge branch 'master' into service-in-experiment
daniddelrio Mar 13, 2023
58dcc48
Merge branch 'master' into service-in-experiment
daniddelrio Mar 17, 2023
1d34b63
Merge branch 'master' into service-in-experiment
daniddelrio Mar 23, 2023
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
49 changes: 46 additions & 3 deletions docs/features/experiment.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ 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 also generated if a weight for that experiment is set.
A Service routing traffic to the Experiment ReplicaSet is also generated if a weight (which requires traffic routing)
OR the Service attribute for that experiment is set.

## Use cases of Experiments

Expand Down Expand Up @@ -51,6 +52,11 @@ spec:
- name: purple
# Number of replicas to run (optional). If omitted, will run a single replica
replicas: 1
# Flag to create Service for this Experiment (optional)
# If omitted, a Service won't be created.
service:
# Name of the Service (optional). If omitted, service: {} would also be acceptable.
name: service-name
selector:
matchLabels:
app: canary-demo
Expand Down Expand Up @@ -119,7 +125,8 @@ spec:
An Experiment is intended to temporarily run one or more templates. The lifecycle of an Experiment
is as follows:

1. Create and scale a ReplicaSet for each pod template specified under `spec.templates`
1. Create and scale a ReplicaSet for each pod template specified under `spec.templates`. If
`service` is specified under a pod template, a Service will also be created for that pod.
2. Wait for all ReplicaSets reach full availability. If a ReplicaSet does not become available
within `spec.progressDeadlineSeconds`, the Experiment will fail. Once available, the Experiment
will transition from the `Pending` state to a `Running` state.
Expand Down Expand Up @@ -250,4 +257,40 @@ to `experiment-baseline`, leaving the remaining 90% of traffic to the old stack.

By default, the generated Service has the name of the ReplicaSet and inherits
ports and selector from the specRef definition. It can be accessed in using the `{{templates.baseline.replicaset.name}}`
or `{{templates.canary.replicaset.name}}` variables respectively.
or `{{templates.canary.replicaset.name}}` variables respectively.



## Experiment Service Creation without Weight

If you don't want to use traffic routing for your Experiments but still want to create
a Service for them, you can set a Service object which takes an optional Name, without
having to set a Weight for them.

```yaml
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
name: guestbook
labels:
app: guestbook
spec:
...
strategy:
canary:
steps:
- experiment:
duration: 1h
templates:
- name: experiment-baseline
specRef: stable
service:
name: test-service
- name: experiment-canary
specRef: canary
```

In the above example, during an update, the first step would start
a baseline vs. canary experiment. This time, a service would be created
for `experiment-baseline` even without setting a weight for it or traffic
routing for the rollout.
5 changes: 5 additions & 0 deletions docs/features/kustomize/rollout_cr_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -13032,6 +13032,11 @@
"type": "object"
},
"service": {
"properties": {
"name": {
"type": "string"
}
},
"type": "object"
},
"template": {
Expand Down
4 changes: 4 additions & 0 deletions docs/features/specification.md
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,10 @@ spec:
templates:
- name: baseline
specRef: stable
# optional, creates a service for the experiment if set
service:
# optional, service: {} is also acceptable if name is not included
name: test-service
- name: canary
specRef: canary
# optional, set the weight of traffic routed to this version
Expand Down
8 changes: 6 additions & 2 deletions experiments/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -301,10 +301,14 @@ func (ec *experimentContext) createTemplateService(template *v1alpha1.TemplateSp
}
}
if (svc == nil || svc.Name != rs.Name) && len(ports) > 0 {
newService, err := ec.CreateService(rs.Name, *template, rs.Labels, ports)
serviceName := rs.Name
if template.Service.Name != "" {
serviceName = template.Service.Name
}
newService, err := ec.CreateService(serviceName, *template, rs.Labels, ports)
if err != nil {
templateStatus.Status = v1alpha1.TemplateStatusError
templateStatus.Message = fmt.Sprintf("Failed to create Service for template '%s': %v", template.Name, err)
templateStatus.Message = fmt.Sprintf("Failed to create Service %s for template '%s': %v", serviceName, template.Name, err)
} else {
ec.templateServices[template.Name] = newService
templateStatus.ServiceName = newService.Name
Expand Down
26 changes: 25 additions & 1 deletion experiments/experiment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ func TestFailServiceCreation(t *testing.T) {
})
newStatus := exCtx.reconcile()
assert.Equal(t, v1alpha1.TemplateStatusError, newStatus.TemplateStatuses[0].Status)
assert.Contains(t, newStatus.TemplateStatuses[0].Message, "Failed to create Service for template 'bad'")
assert.Contains(t, newStatus.TemplateStatuses[0].Message, "Failed to create Service foo-bad for template 'bad'")
assert.Equal(t, v1alpha1.AnalysisPhaseError, newStatus.Phase)
}

Expand Down Expand Up @@ -530,3 +530,27 @@ func TestServiceInheritPortsFromRS(t *testing.T) {
assert.Equal(t, exCtx.templateServices["bar"].Name, "foo-bar")
assert.Equal(t, exCtx.templateServices["bar"].Spec.Ports[0].Port, int32(80))
}

func TestServiceNameSet(t *testing.T) {
templates := generateTemplates("bar")
templates[0].Service = &v1alpha1.TemplateService{
Name: "service-name",
}
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, "service-name")
}
6 changes: 3 additions & 3 deletions experiments/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,17 @@ func (ec *experimentContext) CreateService(serviceName string, template v1alpha1
if err != nil {
// If service already exists, get service and check that it is owned by Experiment Template. Otherwise return error.
if errors.IsAlreadyExists(err) {
svc, err := ec.kubeclientset.CoreV1().Services(ec.ex.Namespace).Get(ctx, service.Name, metav1.GetOptions{})
svc, err := ec.kubeclientset.CoreV1().Services(ec.ex.Namespace).Get(ctx, serviceName, metav1.GetOptions{})
if err != nil {
return nil, err
return nil, fmt.Errorf("did not get existing service with name %s: %v", serviceName, err)
}
controllerRef := metav1.GetControllerOf(svc)
if controllerRef == nil || controllerRef.UID != ec.ex.UID || svc.Annotations == nil || svc.Annotations[v1alpha1.ExperimentNameAnnotationKey] != ec.ex.Name || svc.Annotations[v1alpha1.ExperimentTemplateNameAnnotationKey] != template.Name {
return nil, fmt.Errorf("service %s already exists and is not owned by experiment template %s", serviceName, template.Name)
}
return svc, nil
} else {
return nil, err
return nil, fmt.Errorf("cannot create service: %v %v", err, newService)
}
}
return service, nil
Expand Down
3 changes: 3 additions & 0 deletions manifests/crds/experiment-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,9 @@ spec:
type: object
type: object
service:
properties:
name:
type: string
type: object
template:
properties:
Expand Down
5 changes: 5 additions & 0 deletions manifests/crds/rollout-crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,11 @@ spec:
type: string
type: object
type: object
service:
properties:
name:
type: string
type: object
specRef:
type: string
weight:
Expand Down
8 changes: 8 additions & 0 deletions manifests/install.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -8599,6 +8599,9 @@ spec:
type: object
type: object
service:
properties:
name:
type: string
type: object
template:
properties:
Expand Down Expand Up @@ -11622,6 +11625,11 @@ spec:
type: string
type: object
type: object
service:
properties:
name:
type: string
type: object
specRef:
type: string
weight:
Expand Down
13 changes: 13 additions & 0 deletions pkg/apiclient/rollout/rollout.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,10 @@
"type": "integer",
"format": "int32",
"title": "Weight sets the percentage of traffic the template's replicas should receive"
},
"service": {
"$ref": "#/definitions/github.com.argoproj.argo_rollouts.pkg.apis.rollouts.v1alpha1.TemplateService",
"title": "Service controls the optionally generated service"
}
},
"title": "RolloutExperimentTemplate defines the template used to create experiments for the Rollout's experiment canary step"
Expand Down Expand Up @@ -1785,6 +1789,15 @@
},
"description": "TLSRoute holds the information on the virtual service's TLS/HTTPS routes that are desired to be matched for changing weights."
},
"github.com.argoproj.argo_rollouts.pkg.apis.rollouts.v1alpha1.TemplateService": {
"type": "object",
"properties": {
"name": {
"type": "string",
"title": "Name of the service generated by the experiment"
}
}
},
"github.com.argoproj.argo_rollouts.pkg.apis.rollouts.v1alpha1.TraefikTrafficRouting": {
"type": "object",
"properties": {
Expand Down
5 changes: 4 additions & 1 deletion pkg/apis/rollouts/v1alpha1/experiment_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,10 @@ type TemplateSpec struct {
Service *TemplateService `json:"service,omitempty" protobuf:"bytes,6,opt,name=service"`
}

type TemplateService struct{}
type TemplateService struct {
// Name of the service generated by the experiment
Name string `json:"name,omitempty" protobuf:"bytes,1,opt,name=name"`
}

type TemplateStatusCode string

Expand Down
Loading