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

Fix: the pod fails to run because the certificate of the pod webhook is not up to data after helm upgrading #4420

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

cyclinder
Copy link
Collaborator

@cyclinder cyclinder commented Dec 19, 2024

Using helm templates to rendering podwebhook config rather than controller genarated, which can advoid the webhook cert would be revert when helm upgrade, it can leads to pod fail to start.

Thanks for contributing!

Notice:

What issue(s) does this PR fix:

Fixes #4357

Special notes for your reviewer:

之前 podwebhookconfig 通过 controller 动态生成, 但 helm upgrade 导致 podwebhookconfig 被还原为默认的 value,这会导致 podwebhook 的证书无效,从而导致 pod 无法被创建。此 PR 将 podwebhookconfig 通过 helm templates 统一声明,通过 values 开关控制,避免更新时被还原。

Copy link

codecov bot commented Dec 19, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 79.21%. Comparing base (a63220f) to head (c9075f4).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
pkg/podmanager/pod_webhook.go 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4420      +/-   ##
==========================================
- Coverage   79.43%   79.21%   -0.22%     
==========================================
  Files          54       54              
  Lines        6388     6283     -105     
==========================================
- Hits         5074     4977      -97     
+ Misses       1117     1110       -7     
+ Partials      197      196       -1     
Flag Coverage Δ
unittests 79.21% <0.00%> (-0.22%) ⬇️

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

Files with missing lines Coverage Δ
pkg/podmanager/utils.go 52.77% <ø> (-21.27%) ⬇️
pkg/podmanager/pod_webhook.go 0.00% <0.00%> (ø)

@cyclinder cyclinder changed the title Fix podwebhook would be revert when helm upgrade Fix the pod fail to running due to the podwebhook would be revert when helm upgrade Dec 19, 2024
lou-lan
lou-lan previously approved these changes Dec 20, 2024
@cyclinder
Copy link
Collaborator Author

[Step 9] Install operator: kruise 
/etc/bash.bashrc: line 7: PS1: unbound variable
make[3]: Entering directory '/home/runner/work/spiderpool/spiderpool/test'
/etc/bash.bashrc: line 7: PS1: unbound variable
/etc/bash.bashrc: line 7: PS1: unbound variable
/etc/bash.bashrc: line 7: PS1: unbound variable
/etc/bash.bashrc: line 7: PS1: unbound variable
/etc/bash.bashrc: line 7: PS1: unbound variable
/etc/bash.bashrc: line 7: PS1: unbound variable
/etc/bash.bashrc: line 7: PS1: unbound variable
Makefile:410: warning: overriding recipe for target '@echo'
Makefile:206: warning: ignoring old recipe for target '@echo'
Makefile:410: warning: overriding recipe for target '"check'
Makefile:206: warning: ignoring old recipe for target '"check'
Makefile:410: warning: overriding recipe for target 'cni'
Makefile:206: warning: ignoring old recipe for target 'cni'
Makefile:410: warning: overriding recipe for target 'nodes'
Makefile:206: warning: ignoring old recipe for target 'nodes'
/etc/bash.bashrc: line 7: PS1: unbound variable
add openkruise charts repository...
"openkruise" already exists with the same configuration, skipping
v1.7.2: Pulling from openkruise/kruise-manager
Digest: sha256:15da38958bf9fee89d2730b937c5c3e291390f08dcc05467f737e5fcf6fecfb5
Status: Image is up to date for openkruise/kruise-manager:v1.7.2
docker.io/openkruise/kruise-manager:v1.7.2
Image: "docker.io/openkruise/kruise-manager:v1.7.2" with ID "sha256:414a6b7c8804fa65d20ae7cbcc85572c4181ea85c4240f7ad3bc8039e2929589" not yet present on node "spiderpool1220020736-worker", loading...
Image: "docker.io/openkruise/kruise-manager:v1.7.2" with ID "sha256:414a6b7c8804fa65d20ae7cbcc85572c4181ea85c4240f7ad3bc8039e2929589" not yet present on node "spiderpool1220020736-control-plane", loading...
v0.1.0: Pulling from openkruise/kruise-helm-hook
Digest: sha256:edc7cf9428fd72f9885431a4f0fe4e2e1724f6a8fbd4b592105fbdfdb2a9afdf
Status: Image is up to date for openkruise/kruise-helm-hook:v0.1.0
docker.io/openkruise/kruise-helm-hook:v0.1.0
Image: "openkruise/kruise-helm-hook:v0.1.0" with ID "sha256:ac7049d52f06d5f37229f6abea772d01a0badaee6cb3e71e118b73f98a04280e" not yet present on node "spiderpool1220020736-worker", loading...
Image: "openkruise/kruise-helm-hook:v0.1.0" with ID "sha256:ac7049d52f06d5f37229f6abea772d01a0badaee6cb3e71e118b73f98a04280e" not yet present on node "spiderpool1220020736-control-plane", loading...
# https://github.com/spidernet-io/spiderpool/issues/4396
/etc/bash.bashrc: line 7: PS1: unbound variable
make[3]: *** [Makefile:221: setup_kruise] Error 1
make[3]: Leaving directory '/home/runner/work/spiderpool/spiderpool/test'
make[2]: *** [Makefile:39: kind-init] Error 2
make[2]: Leaving directory '/home/runner/work/spiderpool/spiderpool/test'
make[1]: *** [Makefile:299: e2e_init] Error 2
make[1]: Leaving directory '/home/runner/work/spiderpool/spiderpool'
make: *** [Makefile:323: e2e_init_spiderpool] Error 2

weizhoublue
weizhoublue previously approved these changes Dec 25, 2024
@weizhoublue weizhoublue self-requested a review December 25, 2024 01:57
@weizhoublue
Copy link
Collaborator

weizhoublue commented Dec 25, 2024

I notice you add the label for cherry picking to v0.8 ?
as I remember , this feature only shows up in v0.9 ?

@cyclinder
Copy link
Collaborator Author

Yes, you are right.

@cyclinder cyclinder removed the cherrypick-release-v0.8 Cherry-pick the PR to branch release-v0.8. label Dec 25, 2024
@cyclinder cyclinder dismissed stale reviews from weizhoublue and lou-lan via 3a7780c December 25, 2024 03:29
@cyclinder cyclinder force-pushed the controller/fix_cert branch 6 times, most recently from fc48d82 to 2aa8529 Compare December 26, 2024 06:41
Using helm templates to rendering podwebhook config rather than controller genarated, which can advoid the webhook cert would be revert when helm upgrade, it can leads to pod fail to start.

Signed-off-by: Cyclinder Kuo <[email protected]>
@weizhoublue weizhoublue changed the title Fix the pod fail to running due to the podwebhook would be revert when helm upgrade Fix: the pod fails to run because the certificate of the pod webhook is not up to data after helm upgrading Dec 27, 2024
@weizhoublue weizhoublue merged commit 7fd26d8 into spidernet-io:main Dec 27, 2024
54 of 56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

There is a problem with the spiderpool webhook certificate verification.
3 participants