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

do not return error, when the ds is not found #80798

Merged
merged 1 commit into from Aug 1, 2019

Conversation

@xlgao-zju
Copy link
Member

commented Jul 31, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:
when I upgrade my cluster from 1.13 to 1.14 using kubeadm, I came across the following error:

[upgrade/prepull] Will prepull images for components [kube-apiserver kube-controller-manager kube-scheduler]
[upgrade/prepull] Prepulling image for component kube-scheduler.
[upgrade/prepull] Prepulling image for component kube-apiserver.
[upgrade/prepull] Prepulling image for component kube-controller-manager.
[apiclient] Found 0 Pods for label selector k8s-app=upgrade-prepull-kube-scheduler
[apiclient] Found 0 Pods for label selector k8s-app=upgrade-prepull-kube-controller-manager
[apiclient] Found 0 Pods for label selector k8s-app=upgrade-prepull-kube-apiserver
[apiclient] Found 3 Pods for label selector k8s-app=upgrade-prepull-kube-controller-manager
[apiclient] Found 3 Pods for label selector k8s-app=upgrade-prepull-kube-apiserver
[upgrade/prepull] Prepulled image for component kube-controller-manager.
[upgrade/prepull] Prepulled image for component kube-apiserver.
[upgrade/prepull] Failed prepulled the images for the control plane components error: unable to cleanup the DaemonSet used for prepulling kube-scheduler: daemonsets.apps "upgrade-prepull-kube-scheduler" not found

I think we'd better not to return error, when we intend to delete the ds and the ds is not found.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

kubeadm: avoid double deletion of the upgrade prepull DaemonSet
@xlgao-zju

This comment has been minimized.

Copy link
Member Author

commented Jul 31, 2019

/assign @neolit123

@xlgao-zju

This comment has been minimized.

Copy link
Member Author

commented Jul 31, 2019

/assign @timothysc

Copy link
Member

left a comment

hi @xlgao-zju ,

i see the same error reported here:
kubernetes/kubeadm#1603
kubernetes/kubeadm#1546

so while this PR will probably fix this type of problem, i wish to understand more why this error is actually happening. why are we ending up in a situation trying to delete a DS, that we manage, but that no longer exists?

@neolit123

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

/kind bug
/priority important-longterm

@xlgao-zju

This comment has been minimized.

Copy link
Member Author

commented Jul 31, 2019

why are we ending up in a situation trying to delete a DS, that we manage, but that no longer exists?

yes, we should track this issue, and find out the reason. I will file an issue about this later.
and at the same time, I think we can merge this PR, since we should not return error, when we try to delete something and it can not be found.

@neolit123

This comment has been minimized.

Copy link
Member

commented Jul 31, 2019

@xlgao-zju
i have created this issue to track the problem:
kubernetes/kubeadm#1700

the PR seems fine but we need to:

  • add a release note instead of NONE

kubeadm: avoid double deletion of the upgrade prepull DaemonSet

  • add a TODO above the ...apiclient.DeleteDaemonSetForeground call:
// TODO: The IsNotFound() check is required in cases where the DaemonSet is missing.
// Investigate why this happens: https://github.com/kubernetes/kubeadm/issues/1700
Signed-off-by: Xianglin Gao <xianglin.gxl@alibaba-inc.com>
@xlgao-zju xlgao-zju force-pushed the xlgao-zju:ignore-not-found branch from e5aebda to 1b6ec47 Aug 1, 2019
@xlgao-zju

This comment has been minimized.

Copy link
Member Author

commented Aug 1, 2019

@neolit123 PR is updated, PTAL

@neolit123

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

thanks
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Aug 1, 2019
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Aug 1, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: neolit123, xlgao-zju

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@xlgao-zju

This comment has been minimized.

Copy link
Member Author

commented Aug 1, 2019

/test pull-kubernetes-kubemark-e2e-gce-big

@k8s-ci-robot k8s-ci-robot merged commit 3758426 into kubernetes:master Aug 1, 2019
23 checks passed
23 checks passed
cla/linuxfoundation xlgao-zju authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details
@k8s-ci-robot k8s-ci-robot added this to the v1.16 milestone Aug 1, 2019
@xlgao-zju xlgao-zju deleted the xlgao-zju:ignore-not-found branch Aug 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.