Delete the federation namespace from fcp instead of individual objects #51768

shashidharatd · 2017-09-01T05:59:06Z

What this PR does / why we need it:
This PR simplifies cleanup by deleting the entire namespace instead of individual objects.
This PR is linked to #50543. This may not solve the issue but instead to try an alternative.

Release note:

Delete the federation namespace from control plane instead of individual objects

/assign @madhusudancs

irfanurrehman · 2017-09-01T14:40:24Z

federation/cluster/common.sh

+    # performing a cascading deletion of all the resources in the
+    # namespace. So we sleep a little longer than other resources
+    # before retrying
+    sleep 5


Can we rather wait with a polled query on the ns; and bigger timeout which fails, if it could not delete the ns in that time.
Also I am curious as to why this was not done earlier. (probably some specific reason @madhusudancs )

Can we rather wait with a polled query on the ns; and bigger timeout which fails, if it could not delete the ns in that time.

@irfanurrehman What Shashi has now is actually the preferred method in kubernetes.

Also I am curious as to why this was not done earlier. (probably some specific reason @madhusudancs )

It's mostly for historic reasons. We wanted to be super careful about what we were deleting in the federation-cluster (now federation-system) namespace in the pre-kubefed days. It just carried over to kubefed days.

@madhusudan thanks for the clarifications. Seems ok to me.

irfanurrehman · 2017-09-01T14:41:09Z

/assign @madhusudancs

madhusudancs · 2017-09-06T06:20:30Z

federation/cluster/common.sh

-    sleep 2
+  kube::log::status "Removing namespace \"${FEDERATION_NAMESPACE}\" from \"${FEDERATION_KUBE_CONTEXT}\""
+  # Try deleting until the namespace is completely gone.
+  while $host_kubectl --context="${FEDERATION_KUBE_CONTEXT}" delete namespace "${FEDERATION_NAMESPACE}" >/dev/null 2>&1; do


Do we really need to do this separately at all. Shouldn't the for loop below in line 91 take care of this?

@madhusudancs, the for loop below does not include the federation control plane context and only iterates over federated clusters. may be some resource are leftover, if we just delete the federation api-server and controllermanager pods, but do not delete the objects in the fcp.

I see Ok, fair enough.

madhusudancs · 2017-09-06T06:22:42Z

federation/cluster/common.sh

+    # performing a cascading deletion of all the resources in the
+    # namespace. So we sleep a little longer than other resources
+    # before retrying
+    sleep 5


Can we rather wait with a polled query on the ns; and bigger timeout which fails, if it could not delete the ns in that time.

@irfanurrehman What Shashi has now is actually the preferred method in kubernetes.

Also I am curious as to why this was not done earlier. (probably some specific reason @madhusudancs )

It's mostly for historic reasons. We wanted to be super careful about what we were deleting in the federation-cluster (now federation-system) namespace in the pre-kubefed days. It just carried over to kubefed days.

madhusudancs

/lgtm
/approve

madhusudancs · 2017-09-06T19:01:42Z

federation/cluster/common.sh

-    sleep 2
+  kube::log::status "Removing namespace \"${FEDERATION_NAMESPACE}\" from \"${FEDERATION_KUBE_CONTEXT}\""
+  # Try deleting until the namespace is completely gone.
+  while $host_kubectl --context="${FEDERATION_KUBE_CONTEXT}" delete namespace "${FEDERATION_NAMESPACE}" >/dev/null 2>&1; do


I see Ok, fair enough.

k8s-github-robot · 2017-09-06T19:02:54Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: madhusudancs, shashidharatd

Associated issue: 50543

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

~~federation/OWNERS~~ [madhusudancs]

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

madhusudancs · 2017-09-06T19:21:53Z

@shashidharatd can you also please cherry pick this to 1.8?

fejta-bot · 2017-09-06T21:30:51Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-09-11T13:51:17Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-09-11T20:30:19Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-09-12T07:21:19Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

madhusudancs · 2017-09-13T22:54:19Z

@kubernetes/release-reviewers this is a bug fix. Is there anything else we need to do get this merged?

cc @csbell

shashidharatd · 2017-09-14T03:12:17Z

/test pull-kubernetes-unit

fejta-bot · 2017-09-14T08:21:02Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-09-14T15:21:03Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

fejta-bot · 2017-09-14T23:45:26Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to @fejta).

Review the full test history for this PR.

k8s-ci-robot · 2017-09-15T02:32:25Z

@shashidharatd: The following test failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
pull-kubernetes-e2e-gce-bazel	`320548f`	link	`/test pull-kubernetes-e2e-gce-bazel`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-github-robot · 2017-09-15T02:39:57Z

Automatic merge from submit-queue

wojtek-t · 2017-10-03T08:28:07Z

Cherrypick in #53372

…68-upstream-release-1.7 Automatic merge from submit-queue. Automated cherry pick of #51768 upstream release 1.7 Cherry pick of #51768 on release-1.7. #51768: Delete the federation namespace from fcp instead of individual objects

k8s-cherrypick-bot · 2017-10-03T10:52:10Z

Commit found in the "release-1.7" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

Delete the federation namespace from fcp instead of individual objects

320548f

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 1, 2017

k8s-github-robot assigned ghost and irfanurrehman Sep 1, 2017

k8s-github-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Sep 1, 2017

irfanurrehman reviewed Sep 1, 2017

View reviewed changes

k8s-ci-robot assigned madhusudancs Sep 1, 2017

madhusudancs reviewed Sep 6, 2017

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 6, 2017

k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 6, 2017

shashidharatd mentioned this pull request Sep 6, 2017

cherry-pick: #51768 Delete the federation namespace from fcp instead of individual objects #52041

Merged

madhusudancs added this to the v1.8 milestone Sep 13, 2017

k8s-github-robot merged commit 5a4d1dd into kubernetes:master Sep 15, 2017

shashidharatd mentioned this pull request Sep 27, 2017

Do not leak GCE PD in federation tests #50543

Closed

wojtek-t removed this from the v1.8 milestone Oct 2, 2017

wojtek-t added this to the v1.7 milestone Oct 2, 2017

wojtek-t added the cherrypick-candidate label Oct 2, 2017

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Oct 3, 2017

wojtek-t mentioned this pull request Oct 3, 2017

Automated cherry pick of #51768 upstream release 1.7 #53372

Merged

wojtek-t added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Oct 3, 2017

k8s-cherrypick-bot removed the cherrypick-candidate label Oct 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Delete the federation namespace from fcp instead of individual objects #51768

Delete the federation namespace from fcp instead of individual objects #51768

shashidharatd commented Sep 1, 2017 •

edited by wojtek-t

Loading

irfanurrehman Sep 1, 2017

madhusudancs Sep 6, 2017

irfanurrehman Sep 6, 2017 •

edited

Loading

irfanurrehman commented Sep 1, 2017

madhusudancs Sep 6, 2017

shashidharatd Sep 6, 2017

madhusudancs Sep 6, 2017

madhusudancs Sep 6, 2017

madhusudancs left a comment

madhusudancs Sep 6, 2017

k8s-github-robot commented Sep 6, 2017

madhusudancs commented Sep 6, 2017

fejta-bot commented Sep 6, 2017

fejta-bot commented Sep 11, 2017

fejta-bot commented Sep 11, 2017

fejta-bot commented Sep 12, 2017

madhusudancs commented Sep 13, 2017

shashidharatd commented Sep 14, 2017

fejta-bot commented Sep 14, 2017

fejta-bot commented Sep 14, 2017

fejta-bot commented Sep 14, 2017

k8s-ci-robot commented Sep 15, 2017 •

edited

Loading

k8s-github-robot commented Sep 15, 2017

wojtek-t commented Oct 3, 2017

k8s-cherrypick-bot commented Oct 3, 2017

Delete the federation namespace from fcp instead of individual objects #51768

Delete the federation namespace from fcp instead of individual objects #51768

Conversation

shashidharatd commented Sep 1, 2017 • edited by wojtek-t Loading

irfanurrehman Sep 1, 2017

Choose a reason for hiding this comment

madhusudancs Sep 6, 2017

Choose a reason for hiding this comment

irfanurrehman Sep 6, 2017 • edited Loading

Choose a reason for hiding this comment

irfanurrehman commented Sep 1, 2017

madhusudancs Sep 6, 2017

Choose a reason for hiding this comment

shashidharatd Sep 6, 2017

Choose a reason for hiding this comment

madhusudancs Sep 6, 2017

Choose a reason for hiding this comment

madhusudancs Sep 6, 2017

Choose a reason for hiding this comment

madhusudancs left a comment

Choose a reason for hiding this comment

madhusudancs Sep 6, 2017

Choose a reason for hiding this comment

k8s-github-robot commented Sep 6, 2017

madhusudancs commented Sep 6, 2017

fejta-bot commented Sep 6, 2017

fejta-bot commented Sep 11, 2017

fejta-bot commented Sep 11, 2017

fejta-bot commented Sep 12, 2017

madhusudancs commented Sep 13, 2017

shashidharatd commented Sep 14, 2017

fejta-bot commented Sep 14, 2017

fejta-bot commented Sep 14, 2017

fejta-bot commented Sep 14, 2017

k8s-ci-robot commented Sep 15, 2017 • edited Loading

k8s-github-robot commented Sep 15, 2017

wojtek-t commented Oct 3, 2017

k8s-cherrypick-bot commented Oct 3, 2017

shashidharatd commented Sep 1, 2017 •

edited by wojtek-t

Loading

irfanurrehman Sep 6, 2017 •

edited

Loading

k8s-ci-robot commented Sep 15, 2017 •

edited

Loading