DataDog / extendeddaemonset

Kubernetes Extended Daemonset controller
Apache License 2.0
98 stars 13 forks source link

Fix IsPodAvailable() to wait the PodReady is true #72

Closed clamoriniere closed 3 years ago

clamoriniere commented 3 years ago

What does this PR do?

Fix the function IsPodAvailable() used by the controller to compute the number of available Pods.

Motivation

This behaviour was the origin of a nasty bug with which the controller didn't respect the "MaxUnavailable" parameter.

The controller was considering too many Pods as available, for example, Pods in ImagePullBackoff were considered as available.

Additional Notes

Anything else we should know when reviewing?

Describe your test plan

Create a EDS instance on a multi nodes cluster (you can use the kand and the examples/kind-cluster-configuration.yaml)

apiVersion: datadoghq.com/v1alpha1
kind: ExtendedDaemonSet
metadata:
  name: foo
spec:
  strategy:
    rollingUpdate:
      maxParallelPodCreation: 1
      slowStartIntervalDuration: 1m
      maxUnavailable: 1
  template:
    spec: 
      containers:
      - name: daemon
        image: k8s.gcr.io/pause:3.0
      tolerations:
      - operator: Exists

when the EDS properly deployed, update it with:

apiVersion: datadoghq.com/v1alpha1
kind: ExtendedDaemonSet
metadata:
  name: foo
spec:
  strategy:
    rollingUpdate:
      maxParallelPodCreation: 1
      slowStartIntervalDuration: 1m
      maxUnavailable: 1
  template:
    spec: 
      containers:
      - name: daemon
        image: k8s.gcr.io/pause:42
      tolerations:
      - operator: Exists

The image doesn't exist. so the new pod should in ImagePullBackoff.

wanted behaviour: Only one pod should be updated and in ImagePullBackoff

codecov-io commented 3 years ago

Codecov Report

Merging #72 (e8ceb8c) into master (9df60a1) will increase coverage by 0.09%. The diff coverage is 23.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
+ Coverage   32.26%   32.36%   +0.09%     
==========================================
  Files          37       41       +4     
  Lines        2833     2917      +84     
==========================================
+ Hits          914      944      +30     
- Misses       1830     1882      +52     
- Partials       89       91       +2     
Flag Coverage Δ
unittests 32.36% <23.28%> (+0.09%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/plugin/common/client.go 0.00% <ø> (ø)
pkg/plugin/common/pods.go 0.00% <0.00%> (ø)
pkg/plugin/common/table.go 0.00% <0.00%> (ø)
pkg/plugin/common/utils.go 62.50% <62.50%> (ø)
pkg/controller/utils/pod/pod.go 39.08% <100.00%> (+17.24%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 87217d6...e8ceb8c. Read the comment docs.