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.

Sign up for GitHub

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

Jump to bottom

Adding cascading deletion support to more federation controllers #36330

Merged
merged 3 commits into from
Nov 10, 2016
Merged

Adding cascading deletion support to more federation controllers #36330

merged 3 commits into from
Nov 10, 2016

Conversation

nikhiljindal
Copy link
Contributor

@nikhiljindal nikhiljindal commented Nov 7, 2016 •
edited
Loading

Ref #33612

Adding cascading deletion support for federated daemonsets and ingress.
The code is same as that for namespaces. Just ensuring that DeletionHelper functions are called at right places in these controllers.
e2e tests coming up in another PR.

cc @kubernetes/sig-cluster-federation @caesarxuchao @madhusudancs @mwielgus

federation: Adding support for DeleteOptions.OrphanDependents for federated daemonsets and ingresses. Setting it to false while deleting a federated daemonset or ingress also deletes the corresponding resource from all registered clusters.

This change is Reviewable

@nikhiljindal nikhiljindal added this to the v1.5 milestone Nov 7, 2016
@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. release-note-label-needed labels Nov 7, 2016
@nikhiljindal
Copy link
Contributor Author

Pushed another commit for autogenerated bazel and test owner changes to fix tests

@nikhiljindal nikhiljindal added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Nov 7, 2016
@k8s-github-robot k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 7, 2016
@nikhiljindal nikhiljindal removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 7, 2016
@nikhiljindal
Copy link
Contributor Author

Rebased

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 7, 2016
@nikhiljindal
Copy link
Contributor Author

Updated the code to include fix from #36377 in all these controllers

@k8s-github-robot k8s-github-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 7, 2016
@madhusudancs madhusudancs added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 7, 2016
@madhusudancs
Copy link
Contributor

@nikhiljindal code generally looks good to me. I suppose these are all similar to each other and similar to the changes in the namespace controller?

@nikhiljindal
Copy link
Contributor Author

Yes thats right.
Mostly just copy paste and search replace :)

@nikhiljindal nikhiljindal added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Nov 8, 2016
@nikhiljindal
Copy link
Contributor Author

@k8s-bot unit test this

@nikhiljindal nikhiljindal mentioned this pull request Nov 9, 2016
Merged
@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 9, 2016
@nikhiljindal nikhiljindal added lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Nov 9, 2016
@nikhiljindal
Copy link
Contributor Author

secret test failed. Rebased to include #36463

@nikhiljindal nikhiljindal added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2016
@nikhiljindal
Copy link
Contributor Author

@k8s-bot unit test this

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 9, 2016
@nikhiljindal nikhiljindal removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 9, 2016
@nikhiljindal
Copy link
Contributor Author

Rebased the PR and disabled flaky tests as per #36540

@nikhiljindal nikhiljindal added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 9, 2016
@nikhiljindal
Copy link
Contributor Author

Readding LGTM after rebase

@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit 08da37ee962cfe228c270a113f2b7c214a97baa1. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@nikhiljindal nikhiljindal removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 10, 2016
@nikhiljindal nikhiljindal added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 10, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit 675da90. Full PR test history.

The magic incantation to run this job again is @k8s-bot cvm gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 6b9ce1b into kubernetes:master Nov 10, 2016
k8s-github-robot pushed a commit that referenced this pull request Nov 10, 2016
Merge pull request #36476 from nikhiljindal/deployRS
7d312c2 
Automatic merge from submit-queue

Adding cascading deletion support to federation replicaset and deployments

Forked from #36330

Ref #33612
Adding cascading deletion support for federated replicasets and deployments.

```release-note
federation: Adding support for DeleteOptions.OrphanDependents for federated replicasets and deployments. Setting it to false while deleting a federated replicaset or deployment also deletes the corresponding resource from all registered clusters.
```
@nikhiljindal nikhiljindal mentioned this pull request Nov 11, 2016
Merged
k8s-github-robot pushed a commit that referenced this pull request Nov 19, 2016
Merge pull request #36630 from nikhiljindal/E2edsi
c183664 
Automatic merge from submit-queue

Adding e2e tests for validating cascading deletion of federation resources

Ref #33612

Adding e2e tests for code in #36330 and #36476.

Adding test cases to ensure that cascading deletion happens as expected.
Also adding code to ensure that all resources are cleaned up in AfterEach.

cc @kubernetes/sig-cluster-federation @caesarxuchao @madhusudancs
@chentao1596 chentao1596 mentioned this pull request Dec 5, 2016
Closed
perotinus pushed a commit to kubernetes-retired/cluster-registry that referenced this pull request Sep 2, 2017
Merge pull request #36476 from nikhiljindal/deployRS
5f3522c 
Automatic merge from submit-queue

Adding cascading deletion support to federation replicaset and deployments

Forked from kubernetes/kubernetes#36330

Ref #33612
Adding cascading deletion support for federated replicasets and deployments.

```release-note
federation: Adding support for DeleteOptions.OrphanDependents for federated replicasets and deployments. Setting it to false while deleting a federated replicaset or deployment also deletes the corresponding resource from all registered clusters.
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@madhusudancs madhusudancs

Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.5
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nikhiljindal @madhusudancs @k8s-ci-robot @k8s-github-robot @googlebot