Skip to content

Commit

Permalink
Improve controller logic + fix tests flakiness (#74)
Browse files Browse the repository at this point in the history
  • Loading branch information
clamoriniere committed Jan 19, 2021
1 parent 5d52e25 commit 99ef906
Show file tree
Hide file tree
Showing 34 changed files with 1,470 additions and 535 deletions.
37 changes: 29 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -308,14 +308,6 @@ The Canary deployment can be manually failed. This command will restore the curr

`kubectl-eds canary fail <ExtendedDaemonSet name>`

#### Reset Canary deployment

Following failure of the Canary deployment, the `fail` annotation should be reset with this command.

`kubectl-eds canary reset <ExtendedDaemonSet name>`



### How to migrate from a DaemonSet

If you already have an application running in your cluster with a DaemonSet, it is possible to migrate to an ExtendedDaemonSet with a `smooth` migration path.
Expand Down Expand Up @@ -348,6 +340,10 @@ $ make build
CGO_ENABLED=0 go build -i -installsuffix cgo -ldflags '-w' -o controller ./cmd/manager/main.go
```

### Implementation documentation

* [Reconcile loops interactions](./docs/canary-worflows.md)

### How to test it

### Custom image
Expand All @@ -369,6 +365,31 @@ ok github.com/DataDog/extendeddaemonset/controllers/extendeddaemonsetreplic
ok github.com/DataDog/extendeddaemonset/pkg/controller/utils 1.015s coverage: 100.0% of statements
```

##### controller-runtime envtest

This project is using the `controller-runtime` [envtest](https://book.kubebuilder.io/reference/envtest.html) to test reconcile controllers loop against an API-Server dynamically started to run the tests.
One advantage of using the controller-runtime `envtest` is that tests run faster compared to tests that run against a real k8s cluster. The downside is: only the `API-Server` is running but not the other controllers, so resources such as Pods are not updated by a Kubelet. You can find more information about the `envtest` limitation [here](https://book.kubebuilder.io/reference/envtest.html#testing-considerations).

these test are located in `/controllers/extendeddaemonset_test.go`

#### end2end test

End2end tests are also present. Unlike the tests that use the `envtest`, the e2e tests need to have access to a running kubernetes cluster.
The envvar KUBECONFIG should be set in the terminal where the `make e2e` is executed.

[Kind](https://kind.sh/) is a great solution to start a multi-nodes cluster locally.

```console
$ kind create cluster --config examples/kind-cluster-configuration.yaml
cluster created
$ make e2e
Ran 12 of 12 Specs in 242.249 seconds
SUCCESS! -- 12 Passed | 0 Failed | 0 Pending | 0 Skipped
--- PASS: TestAPIs (242.25s)
PASS
ok github.com/DataDog/extendeddaemonset/controllers 242.686s
```

### Linter validation

To use the linter, run:
Expand Down
37 changes: 37 additions & 0 deletions api/v1alpha1/extendeddaemonset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,38 @@ const (
ExtendedDaemonSetStatusReasonUnknown ExtendedDaemonSetStatusReason = "Unknown"
)

// ExtendedDaemonSetConditionType type use to represent a ExtendedDaemonSetR condition
type ExtendedDaemonSetConditionType string

const (
// ConditionTypeEDSReconcileError the controller wasn't able to run properly the reconcile loop with this ExtendedDaemonSet
ConditionTypeEDSReconcileError ExtendedDaemonSetConditionType = "ReconcileError"
// ConditionTypeEDSCanaryPaused ExtendedDaemonSet is in canary mode
ConditionTypeEDSCanaryPaused ExtendedDaemonSetConditionType = "Canary-Paused"
// ConditionTypeEDSCanaryFailed ExtendedDaemonSetis in canary mode
ConditionTypeEDSCanaryFailed ExtendedDaemonSetConditionType = "Canary-Failed"
)

// ExtendedDaemonSetCondition describes the state of a ExtendedDaemonSet at a certain point.
type ExtendedDaemonSetCondition struct {
// Type of ExtendedDaemonSetReplicaSet condition.
Type ExtendedDaemonSetConditionType `json:"type"`
// Status of the condition, one of True, False, Unknown.
Status corev1.ConditionStatus `json:"status"`
// Last time the condition transitioned from one status to another.
// +optional
LastTransitionTime metav1.Time `json:"lastTransitionTime,omitempty"`
// Last time the condition was updated.
// +optional
LastUpdateTime metav1.Time `json:"lastUpdateTime,omitempty"`
// The reason for the condition's last transition.
// +optional
Reason string `json:"reason,omitempty"`
// A human readable message indicating details about the transition.
// +optional
Message string `json:"message,omitempty"`
}

// ExtendedDaemonSetStatus defines the observed state of ExtendedDaemonSet
// +k8s:openapi-gen=true
type ExtendedDaemonSetStatus struct {
Expand All @@ -152,6 +184,11 @@ type ExtendedDaemonSetStatus struct {
// Reason provides an explanation for canary deployment autopause
// +optional
Reason ExtendedDaemonSetStatusReason `json:"reason,omitempty"`

// Conditions Represents the latest available observations of a DaemonSet's current state.
// +listType=map
// +listMapKey=type
Conditions []ExtendedDaemonSetCondition `json:"conditions,omitempty"`
}

// ExtendedDaemonSetStatusCanary defines the observed state of ExtendedDaemonSet canary deployment
Expand Down
4 changes: 4 additions & 0 deletions api/v1alpha1/extendeddaemonsetreplicaset_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ const (
ConditionTypePodCannotStart ExtendedDaemonSetReplicaSetConditionType = "PodCannotStart"
// ConditionTypeLastFullSync last time the ExtendedDaemonSetReplicaSet sync when to the end of the reconcile function
ConditionTypeLastFullSync ExtendedDaemonSetReplicaSetConditionType = "LastFullSync"
// ConditionTypeCanaryPaused ExtendedDaemonSetReplicaSet is in canary mode
ConditionTypeCanaryPaused ExtendedDaemonSetReplicaSetConditionType = "Canary-Paused"
// ConditionTypeCanaryFailed ExtendedDaemonSetReplicaSet is in canary mode
ConditionTypeCanaryFailed ExtendedDaemonSetReplicaSetConditionType = "Canary-Failed"
)

// ExtendedDaemonSetReplicaSet is the Schema for the extendeddaemonsetreplicasets API
Expand Down
24 changes: 24 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

23 changes: 22 additions & 1 deletion api/v1alpha1/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

37 changes: 37 additions & 0 deletions config/crd/bases/v1/datadoghq.com_extendeddaemonsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6183,6 +6183,43 @@ spec:
required:
- replicaSet
type: object
conditions:
description: Conditions Represents the latest available observations
of a DaemonSet's current state.
items:
description: ExtendedDaemonSetCondition describes the state of a
ExtendedDaemonSet at a certain point.
properties:
lastTransitionTime:
description: Last time the condition transitioned from one status
to another.
format: date-time
type: string
lastUpdateTime:
description: Last time the condition was updated.
format: date-time
type: string
message:
description: A human readable message indicating details about
the transition.
type: string
reason:
description: The reason for the condition's last transition.
type: string
status:
description: Status of the condition, one of True, False, Unknown.
type: string
type:
description: Type of ExtendedDaemonSetReplicaSet condition.
type: string
required:
- status
- type
type: object
type: array
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
current:
format: int32
type: integer
Expand Down
34 changes: 34 additions & 0 deletions config/crd/bases/v1beta1/datadoghq.com_extendeddaemonsets.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -6121,6 +6121,40 @@ spec:
required:
- replicaSet
type: object
conditions:
description: Conditions Represents the latest available observations
of a DaemonSet's current state.
items:
description: ExtendedDaemonSetCondition describes the state of a ExtendedDaemonSet
at a certain point.
properties:
lastTransitionTime:
description: Last time the condition transitioned from one status
to another.
format: date-time
type: string
lastUpdateTime:
description: Last time the condition was updated.
format: date-time
type: string
message:
description: A human readable message indicating details about
the transition.
type: string
reason:
description: The reason for the condition's last transition.
type: string
status:
description: Status of the condition, one of True, False, Unknown.
type: string
type:
description: Type of ExtendedDaemonSetReplicaSet condition.
type: string
required:
- status
- type
type: object
type: array
current:
format: int32
type: integer
Expand Down
119 changes: 119 additions & 0 deletions controllers/extendeddaemonset/conditions/update.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
// Unless explicitly stated otherwise all files in this repository are licensed
// under the Apache License Version 2.0.
// This product includes software developed at Datadog (https://www.datadoghq.com/).
// Copyright 2016-2019 Datadog, Inc.

package conditions

import (
corev1 "k8s.io/api/core/v1"

metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

datadoghqv1alpha1 "github.com/DataDog/extendeddaemonset/api/v1alpha1"
)

// NewExtendedDaemonSetCondition returns new ExtendedDaemonSetCondition instance
func NewExtendedDaemonSetCondition(conditionType datadoghqv1alpha1.ExtendedDaemonSetConditionType, conditionStatus corev1.ConditionStatus, now metav1.Time, reason, message string, supportLastUpdate bool) datadoghqv1alpha1.ExtendedDaemonSetCondition {
return datadoghqv1alpha1.ExtendedDaemonSetCondition{
Type: conditionType,
Status: conditionStatus,
LastUpdateTime: now,
LastTransitionTime: now,
Reason: reason,
Message: message,
}
}

// UpdateConditionOptions used to tune how te condition can be updated
// also if the condition doesn't exist yet, it will create it.
type UpdateConditionOptions struct {
// IgnoreFalseConditionIfNotExist used to avoid creating the condition when this condition if false.
// If `IgnoreFalseConditionIfNotExist == `true`, the condition will not be created if the status is equal to `corev1.ConditionFalse`
IgnoreFalseConditionIfNotExist bool
// SupportLastUpdate is an option to avoid updating the `LastUpdateTime` during every reconcile loop.
// This option is useful when only `LastTransitionTime` is the important information.
SupportLastUpdate bool
}

// UpdateExtendedDaemonSetStatusCondition used to update a specific ExtendedDaemonSetConditionType
func UpdateExtendedDaemonSetStatusCondition(status *datadoghqv1alpha1.ExtendedDaemonSetStatus, now metav1.Time, t datadoghqv1alpha1.ExtendedDaemonSetConditionType, conditionStatus corev1.ConditionStatus, reason, desc string, options *UpdateConditionOptions) {

// manage options
var writeFalseIfNotExist, supportLastUpdate bool
if options != nil {
writeFalseIfNotExist = options.IgnoreFalseConditionIfNotExist
supportLastUpdate = options.SupportLastUpdate
}

idCondition := getIndexForConditionType(status, t)
if idCondition >= 0 {
if status.Conditions[idCondition].Status != conditionStatus {
status.Conditions[idCondition].LastTransitionTime = now
status.Conditions[idCondition].Status = conditionStatus
status.Conditions[idCondition].LastUpdateTime = now
}
if supportLastUpdate {
status.Conditions[idCondition].LastUpdateTime = now
}
if conditionStatus == corev1.ConditionTrue {
status.Conditions[idCondition].Message = desc
status.Conditions[idCondition].Reason = reason
}
} else if conditionStatus == corev1.ConditionTrue || writeFalseIfNotExist {
// Only add if the condition is True
status.Conditions = append(status.Conditions, NewExtendedDaemonSetCondition(t, conditionStatus, now, reason, desc, supportLastUpdate))
}
}

// UpdateErrorCondition used to update the ExtendedDaemonSet status error condition
func UpdateErrorCondition(status *datadoghqv1alpha1.ExtendedDaemonSetStatus, now metav1.Time, err error, desc string) {
options := UpdateConditionOptions{
IgnoreFalseConditionIfNotExist: false,
SupportLastUpdate: true,
}
if err != nil {
UpdateExtendedDaemonSetStatusCondition(status, now, datadoghqv1alpha1.ConditionTypeEDSReconcileError, corev1.ConditionTrue, "", desc, &options)
} else {
UpdateExtendedDaemonSetStatusCondition(status, now, datadoghqv1alpha1.ConditionTypeEDSReconcileError, corev1.ConditionFalse, "", desc, &options)
}
}

func getIndexForConditionType(status *datadoghqv1alpha1.ExtendedDaemonSetStatus, t datadoghqv1alpha1.ExtendedDaemonSetConditionType) int {
if status == nil {
return -1
}
for i, condition := range status.Conditions {
if condition.Type == t {
return i
}
}
return -1
}

// GetExtendedDaemonSetStatusCondition return the condition struct corresponding to the ExtendedDaemonSetConditionType provided in argument.
// return nil if not found
func GetExtendedDaemonSetStatusCondition(status *datadoghqv1alpha1.ExtendedDaemonSetStatus, t datadoghqv1alpha1.ExtendedDaemonSetConditionType) *datadoghqv1alpha1.ExtendedDaemonSetCondition {
idCondition := getIndexForConditionType(status, t)
if idCondition == -1 {
return nil
}
return &status.Conditions[idCondition]
}

// IsConditionTrue check if a condition is True. It not set return False.
func IsConditionTrue(status *datadoghqv1alpha1.ExtendedDaemonSetStatus, t datadoghqv1alpha1.ExtendedDaemonSetConditionType) bool {
cond := GetExtendedDaemonSetStatusCondition(status, t)
if cond != nil && cond.Status == corev1.ConditionTrue {
return true
}
return false
}

// BoolToCondition convert bool to corev1.ConditionStatus
func BoolToCondition(value bool) corev1.ConditionStatus {
if value {
return corev1.ConditionTrue
}
return corev1.ConditionFalse
}
Loading

0 comments on commit 99ef906

Please sign in to comment.