[Federation] Update namespace support to use the sync controller

[marun]

This PR moves namespaces to use the sync controller.

cc: @kubernetes/sig-federation-pr-reviews

[marun]

@nikhiljindal The deletion check in the crud integration test is failing. I think the finalizer is being removed in the federated namespace, but removal in member clusters is failing with the following error:

E0621 22:03:04.526227   10013 controller.go:355] Failed to delete namespace "test-namespace-4vgtb": failed to execute updates for obj test-namespace-4vgtb: Operation cannot be fulfilled on namespaces "test-namespace-4vgtb": The system is ensuring all content is removed from this namespace.  Upon completion, this namespace will automatically be purged by the system.

How is the finalizer intended to be removed from member clusters? Is there a controller that does this, and maybe it's not running on the member clusters configured for the integration tests?

[nikhiljindal]

The finalizer should not be propagated to resources in underlying clusters.

Note that namespaces is special in that NamespaceSpec has its own finalizers as well: https://github.com/kubernetes/kubernetes/blob/master/pkg/api/v1/types.go#L3601 (we added finalizer support to namespaces first before adding the generic field in ObjectMeta for all resources).

[marun]

@nikhiljindal The kubernetes finalizer is the one I was talking about. I'm assuming a controller in a member cluster is responsible for removing it?

Edit: Looks like the NamespaceController will need to be running.

[nikhiljindal]

Yes i was talking about our federation finalizers. We do not propagate them to underlying clusters.

You are right, NamespaceController running in all clusters remove the kubernetes finalizer. Its used to ensure that all resources in a namespace are deleted when the namespace is deleted.

[marun]

A namespace controller is now running for each member cluster in the federation fixture.

I'm reusing the namespace controller fixture from test/e2e_node/services for now. I'm assuming this should be extracted for reuse? Is it just me, or is e2e_node more in keeping with integration testing than e2e?

cc: @kubernetes/sig-node-pr-reviews @kubernetes/sig-testing-misc

[marun]

@emaildanwilson I noticed while working on this that the negative selector integration test takes 30s. I think I was the one suggesting a long timeout was a good idea, but on second thought I think it makes sense to break the selector tests into their own test so the CRUD tests can be run faster while working on the sync controller and the adapters.

[marun]

/test pull-kubernetes-unit

Flakey integration test? Yuck!

[marun]

wth. git rebase fail!

[perotinus]

Overall, this LGTM.

[emaildanwilson]

@marun yes, that sounds annoying to deal with. I'll get it moved out into a separate test.

[marun]

rebased

[marun]

/test pull-kubernetes-e2e-gce-etcd3

[marun]

@perotinus bump

[marun]

rebased

[csbell]

This appears to have been moved into the MemberCluster struct instead.

[marun]

Done

[csbell]

/lgtm
/approve no-issue

[k8s-github-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csbell, marun

Associated issue requirement bypassed by: csbell

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 [csbell]
• test/OWNERS [marun]

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

[marun]

/test pull-kubernetes-federation-e2e-gce

[csbell]

/test pull-kubernetes-federation-e2e-gce

[k8s-github-robot]

Automatic merge from submit-queue (batch tested with PRs 47999, 47890)