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 repeat delete service and pod #998

Merged
merged 1 commit into from
May 15, 2019
Merged

Conversation

wackxu
Copy link
Contributor

@wackxu wackxu commented May 14, 2019

fix #997

/assign @gaocegege @richardsliu


This change is Reviewable

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.744% when pulling c352766 on wackxu:fixdel into 7432acb on kubeflow:master.

@ChanYiLin
Copy link
Member

/lgtm

Copy link
Member

@gaocegege gaocegege left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

Thanks for your contribution!

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gaocegege

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

@k8s-ci-robot k8s-ci-robot merged commit 63de5cb into kubeflow:master May 15, 2019
@wackxu wackxu deleted the fixdel branch May 15, 2019 01:26
Jeffwan added a commit to Jeffwan/common that referenced this pull request Apr 15, 2020
Kubernetes pkg/controller/controller_utils.go doesn’t have this logic. This is from kubeflow/training-operator/pull/998. I think it’s safe to keep the logic here.
k8s-ci-robot pushed a commit to kubeflow/common that referenced this pull request Apr 20, 2020
* Add PodControlInterface and FakePodControl

* Add comments for service event reasons

* Skip deleting pods/services already in termination

Kubernetes pkg/controller/controller_utils.go doesn’t have this logic. This is from kubeflow/training-operator/pull/998. I think it’s safe to keep the logic here.

* Remove Kubernetes controller dependency

Use `KeyFunc` instead of k8s.io/kubernetes/pkg/controller.KeyFunc

* Add Base and PodControllerRefManager

Current implementation use PodControllerRefManager from `k8s.io/kubernetes/pkg/controller` and self-implemented `ServiceControllerRefManager` which brings some problems.

1. Pod impl may change along with k8s upgrade, service impl doesn’t
2. It’s not helping us remove Kubernetes direct dependency.
3. It’s not that straighforward for maintainers and contributors.

This change make sure we folk everything we need and remove ref_manager dependency.

* Rename service_ref_manager to controller_ref_manager

Since this file contains both BaseControllerRefManager, PodControllerRefManager and ServiceControllerRefManager. It makes sense to rename it to controller_ref_manager.go

* Address code review feedbacks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tf-operator delete pod and service repeatedly
6 participants