sidecarset forbid updating of sidecar container name #937
Conversation
|
Welcome @adairxie! It looks like this is your first PR to openkruise/kruise 🎉 |
Codecov Report
@@ Coverage Diff @@
## master #937 +/- ##
==========================================
+ Coverage 49.42% 49.45% +0.02%
==========================================
Files 119 119
Lines 11306 11316 +10
==========================================
+ Hits 5588 5596 +8
- Misses 4857 4859 +2
Partials 861 861
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
adairxie
force-pushed
the
container-name-change-bug
branch
from
7788963 to
b6117c2
Compare
|
@adairxie Thanks. The DCO failed. Please git commit --amend -s to signoff yourself and force push again. |
Signed-off-by: xiehui <615810790@qq.com>
adairxie
force-pushed
the
container-name-change-bug
branch
from
b6117c2 to
ab17595
Compare
|
/lgtm |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: FillZpp The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: xiehui <615810790@qq.com> Signed-off-by: Liu Zhenwei <zwliu@thoughtworks.com>
Ⅰ. Describe what this PR does
This PR will forbid updating of sidecar container name in admission web hook. It will crash openkruise server when changing sidecar container's name under version v1.0.1，as shown in the figure below:
I0316 15:08:49.731232 1 sidecarset_controller.go:143] begin to process sidecarset cube-service-trunk-istio-sidecarset for reconcile E0316 15:08:49.731487 1 runtime.go:78] Observed a panic: "invalid memory address or nil pointer dereference" (runtime error: invalid memory address or nil pointer dereference) goroutine 1057 [running]: k8s.io/apimachinery/pkg/util/runtime.logPanic(0x1c4fe60, 0x2e5e7d0) /workspace/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:74 +0x95 k8s.io/apimachinery/pkg/util/runtime.HandleCrash(0x0, 0x0, 0x0) /workspace/vendor/k8s.io/apimachinery/pkg/util/runtime/runtime.go:48 +0x86 panic(0x1c4fe60, 0x2e5e7d0) /usr/local/go/src/runtime/panic.go:965 +0x1b9 github.com/openkruise/kruise/pkg/controller/sidecarset.flipPodSidecarContainerDo(0x218af68, 0xc014f80398, 0xc005651c00) /workspace/pkg/controller/sidecarset/sidecarset_hotupgrade.go:81 +0x310 github.com/openkruise/kruise/pkg/controller/sidecarset.(*Processor).flipPodSidecarContainer.func1(0xc015101548, 0x40e278) /workspace/pkg/controller/sidecarset/sidecarset_hotupgrade.go:49 +0x5c k8s.io/client-go/util/retry.OnError.func1(0xc015101590, 0x40e278, 0x1) /workspace/vendor/k8s.io/client-go/util/retry/util.go:51 +0x3c k8s.io/apimachinery/pkg/util/wait.runConditionWithCrashProtection(0xc015101640, 0xc015101600, 0x0, 0x0) /workspace/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:211 +0x69 k8s.io/apimachinery/pkg/util/wait.ExponentialBackoff(0x989680, 0x4014000000000000, 0x3fb999999999999a, 0x4, 0x0, 0xc015101640, 0x0, 0x2) /workspace/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:399 +0x55 k8s.io/client-go/util/retry.OnError(0x989680, 0x4014000000000000, 0x3fb999999999999a, 0x4, 0x0, 0x1f97a98, 0xc0151016e8, 0x2, 0x2) /workspace/vendor/k8s.io/client-go/util/retry/util.go:50 +0xa6 k8s.io/client-go/util/retry.RetryOnConflict(...) /workspace/vendor/k8s.io/client-go/util/retry/util.go:104 github.com/openkruise/kruise/pkg/controller/sidecarset.(*Processor).flipPodSidecarContainer(0xc00bbad0b0, 0x218af68, 0xc014f80398, 0xc005651000, 0x0, 0xc0151017b0) /workspace/pkg/controller/sidecarset/sidecarset_hotupgrade.go:47 +0x107 github.com/openkruise/kruise/pkg/controller/sidecarset.(*Processor).flipHotUpgradingContainers(0xc00bbad0b0, 0x218af68, 0xc014f80398, 0xc014f803d8, 0x1, 0x1, 0x0, 0x1) /workspace/pkg/controller/sidecarset/sidecarset_hotupgrade.go:36 +0x145 github.com/openkruise/kruise/pkg/controller/sidecarset.(*Processor).UpdateSidecarSet(0xc00bbad0b0, 0xc00133a600, 0x0, 0x0, 0x0, 0x0) /workspace/pkg/controller/sidecarset/sidecarset_processor.go:107 +0x7be github.com/openkruise/kruise/pkg/controller/sidecarset.(*ReconcileSidecarSet).Reconcile(0xc00bbad0e0, 0x21728f8, 0xc009fd4660, 0x0, 0x0, 0xc00f5fa2a0, 0x29, 0xc009fd4660, 0xc00003a000, 0x1d38f40, ...) /workspace/pkg/controller/sidecarset/sidecarset_controller.go:144 +0x259 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).reconcileHandler(0xc002632460, 0x2172850, 0xc000c81580, 0x1cdee40, 0xc00d619a00) /workspace/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:298 +0x30d sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem(0xc002632460, 0x2172850, 0xc000c81580, 0x0) /workspace/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:253 +0x205 sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func1.2(0x2172850, 0xc000c81580) /workspace/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:216 +0x4a k8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext.func1() /workspace/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:185 +0x37 k8s.io/apimachinery/pkg/util/wait.BackoffUntil.func1(0xc00069a750) /workspace/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:155 +0x5f k8s.io/apimachinery/pkg/util/wait.BackoffUntil(0xc015101f50, 0x2131c20, 0xc00ae44f30, 0xc000c81501, 0xc001c8c000) /workspace/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:156 +0x9b k8s.io/apimachinery/pkg/util/wait.JitterUntil(0xc00069a750, 0x3b9aca00, 0x0, 0xc005b50e01, 0xc001c8c000) /workspace/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:133 +0x98 k8s.io/apimachinery/pkg/util/wait.JitterUntilWithContext(0x2172850, 0xc000c81580, 0xc00c2b5cb0, 0x3b9aca00, 0x0, 0x1f99601) /workspace/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:185 +0xa6 k8s.io/apimachinery/pkg/util/wait.UntilWithContext(0x2172850, 0xc000c81580, 0xc00c2b5cb0, 0x3b9aca00) /workspace/vendor/k8s.io/apimachinery/pkg/util/wait/wait.go:99 +0x57 created by sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func1 /workspace/vendor/sigs.k8s.io/controller-runtime/pkg/internal/controller/controller.go:213 +0x40dAlthough it has been fixed by the function sidecarcontrol.IsPodConsistentWithSidecarSet in the v1.0.1, but it will cause sidecarset not to match any existing pod. In a way, it can also lead to orphan sidecar container.
Therefore, we should explicitly refuse to update the sidecar container name in the webhook.
Ⅱ. Does this pull request fix one issue?
NONE
Ⅲ. Describe how to verify it
Under the version of 1.0.1, you can crash openkruise server by changing the sidecar container's name in the sidecarset, note that the sidecar container's upgrade strategy must be hotupgrade.
At the version of 1.0.1, after you change your sidecar container name, the sidecarset will not matching any pods. You can verify it by check sidecarset status.
Ⅳ. Special notes for reviews