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

Add multicluster unlink command #4707

Closed
adleong opened this issue Jul 2, 2020 · 1 comment · Fixed by #4802
Closed

Add multicluster unlink command #4707

adleong opened this issue Jul 2, 2020 · 1 comment · Fixed by #4802

Comments

@adleong
Copy link
Member

adleong commented Jul 2, 2020

Add a linkerd multicluster unlink command which uninstalls an individual multicluster link.

This command requires a --cluster-name flag, just like linkerd multicluster link does.

This command outputs manifests of all resources associated with the Link so that they can be piped to kubectl delete.

However, a naive implementation may result in a race condition where mirror services are deleted before the service mirror controller and the service mirror controller re-creates them before itself getting deleted.

A more sophisticated approach would be for the unlink command to send a signal (such as an HTTP request to the admin port) to the service mirror controller telling it to clean up all of the resources that it manages (mirror services, gateway mirror, and their endpoints) and the block until this is complete. Only once this cleanup is complete will the unlink command then output manifests for the Link resource, the service-mirror-controller, the cluster credentials secret, and the associated RBAC (i.e. all manifests created by linkerd multicluster link).

@alpeb
Copy link
Member

alpeb commented Jul 3, 2020

How about using owner references? All the objects created are marked to be owned by the service mirror controller, so in theory, deleting the service mirror controller should automatically delete everything.

adleong added a commit that referenced this issue Jul 23, 2020
This PR removes the service mirror controller from `linkerd mc install` to `linkerd mc link`, as described in linkerd/rfc#31.  For fuller context, please see that RFC.

Basic multicluster functionality works here including:
* `linkerd mc install` installs the Link CRD but not any service mirror controllers
* `linkerd mc link` creates a Link resource and installs a service mirror controller which uses that Link
* The service mirror controller creates and manages mirror services, a gateway mirror, and their endpoints.
* The `linkerd mc gateways` command lists all linked target clusters, their liveliness, and probe latences.
* The `linkerd check` multicluster checks have been updated for the new architecture.  Several checks have been rendered obsolete by the new architecture and have been removed.

The following are known issues requiring further work:
* the service mirror controller uses the existing `mirror.linkerd.io/gateway-name` and `mirror.linkerd.io/gateway-ns` annotations to select which services to mirror.  it does not yet support configuring a label selector.
* an unlink command is needed for removing multicluster links: see #4707
* an mc uninstall command is needed for uninstalling the multicluster addon: see #4708

Signed-off-by: Alex Leong <[email protected]>
han-so1omon pushed a commit to han-so1omon/linkerd2 that referenced this issue Jul 28, 2020
This PR removes the service mirror controller from `linkerd mc install` to `linkerd mc link`, as described in linkerd/rfc#31.  For fuller context, please see that RFC.

Basic multicluster functionality works here including:
* `linkerd mc install` installs the Link CRD but not any service mirror controllers
* `linkerd mc link` creates a Link resource and installs a service mirror controller which uses that Link
* The service mirror controller creates and manages mirror services, a gateway mirror, and their endpoints.
* The `linkerd mc gateways` command lists all linked target clusters, their liveliness, and probe latences.
* The `linkerd check` multicluster checks have been updated for the new architecture.  Several checks have been rendered obsolete by the new architecture and have been removed.

The following are known issues requiring further work:
* the service mirror controller uses the existing `mirror.linkerd.io/gateway-name` and `mirror.linkerd.io/gateway-ns` annotations to select which services to mirror.  it does not yet support configuring a label selector.
* an unlink command is needed for removing multicluster links: see linkerd#4707
* an mc uninstall command is needed for uninstalling the multicluster addon: see linkerd#4708

Signed-off-by: Alex Leong <[email protected]>
han-so1omon pushed a commit to han-so1omon/linkerd2 that referenced this issue Jul 28, 2020
This PR removes the service mirror controller from `linkerd mc install` to `linkerd mc link`, as described in linkerd/rfc#31.  For fuller context, please see that RFC.

Basic multicluster functionality works here including:
* `linkerd mc install` installs the Link CRD but not any service mirror controllers
* `linkerd mc link` creates a Link resource and installs a service mirror controller which uses that Link
* The service mirror controller creates and manages mirror services, a gateway mirror, and their endpoints.
* The `linkerd mc gateways` command lists all linked target clusters, their liveliness, and probe latences.
* The `linkerd check` multicluster checks have been updated for the new architecture.  Several checks have been rendered obsolete by the new architecture and have been removed.

The following are known issues requiring further work:
* the service mirror controller uses the existing `mirror.linkerd.io/gateway-name` and `mirror.linkerd.io/gateway-ns` annotations to select which services to mirror.  it does not yet support configuring a label selector.
* an unlink command is needed for removing multicluster links: see linkerd#4707
* an mc uninstall command is needed for uninstalling the multicluster addon: see linkerd#4708

Signed-off-by: Alex Leong <[email protected]>
Signed-off-by: Eric Solomon <[email protected]>
adleong added a commit that referenced this issue Aug 4, 2020
Fixes #4707 

In order to remove a multicluster link, we add a `linkerd multicluster unlink` command which produces the yaml necessary to delete all of the resources associated with a `linkerd multicluster link`.  These are:
* the link resource
* the service mirror controller deployment
* the service mirror controller's RBAC
* the probe gateway mirror for this link
* all mirror services for this link

This command follows the same pattern as the `linkerd uninstall` command in that its output is expected to be piped to `kubectl delete`.  The typical usage of this command is:

```
linkerd --context=source multicluster unlink --cluster-name=foo | kubectl --context=source delete -f -
```

This change also fixes the shutdown lifecycle of the service mirror controller by properly having it listen for the shutdown signal and exit its main loop.

A few alternative designs were considered:

I investigated using owner references as suggested [here](#4707 (comment)) but it turns out that owner references must refer to resources in the same namespace (or to cluster scoped resources).  This was not feasible here because a service mirror controller can create mirror services in many different namespaces.

I also considered having the service mirror controller delete the mirror services that it created during its own shutdown.  However, this could lead to scenarios where the controller is killed before it finishes deleting the services that it created.  It seemed more reliable to have all the deletions happen from `kubectl delete`.  Since this is the case, we avoid having the service mirror controller delete mirror services, even when the link is deleted, to avoid the race condition where the controller and CLI both attempt to delete the same mirror services and one of them fails with a potentially alarming error message.

Signed-off-by: Alex Leong <[email protected]>
mvaal pushed a commit to mvaal/linkerd2 that referenced this issue Aug 5, 2020
Fixes linkerd#4707 

In order to remove a multicluster link, we add a `linkerd multicluster unlink` command which produces the yaml necessary to delete all of the resources associated with a `linkerd multicluster link`.  These are:
* the link resource
* the service mirror controller deployment
* the service mirror controller's RBAC
* the probe gateway mirror for this link
* all mirror services for this link

This command follows the same pattern as the `linkerd uninstall` command in that its output is expected to be piped to `kubectl delete`.  The typical usage of this command is:

```
linkerd --context=source multicluster unlink --cluster-name=foo | kubectl --context=source delete -f -
```

This change also fixes the shutdown lifecycle of the service mirror controller by properly having it listen for the shutdown signal and exit its main loop.

A few alternative designs were considered:

I investigated using owner references as suggested [here](linkerd#4707 (comment)) but it turns out that owner references must refer to resources in the same namespace (or to cluster scoped resources).  This was not feasible here because a service mirror controller can create mirror services in many different namespaces.

I also considered having the service mirror controller delete the mirror services that it created during its own shutdown.  However, this could lead to scenarios where the controller is killed before it finishes deleting the services that it created.  It seemed more reliable to have all the deletions happen from `kubectl delete`.  Since this is the case, we avoid having the service mirror controller delete mirror services, even when the link is deleted, to avoid the race condition where the controller and CLI both attempt to delete the same mirror services and one of them fails with a potentially alarming error message.

Signed-off-by: Alex Leong <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants