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

Jump to bottom

Better support for operator style applications #303

Open
huyhg opened this issue Jan 22, 2019 · 4 comments
Open

Better support for operator style applications #303

huyhg opened this issue Jan 22, 2019 · 4 comments
Labels
FR Feature Request

Comments

@huyhg
Copy link
Contributor

huyhg commented Jan 22, 2019

  • Installation of CRDs.
  • Potentially multiple namespaces.
  • Controller/CRDs should be GC'ed when the Application is deleted.
@huyhg huyhg added the FR Feature Request label Jan 22, 2019
@RJPercival
Copy link

From my testing with the etcd-operator, I found that the main problem right now is the deployer. It doesn't wait for the operator to install its CRD, and therefore fails because the operator-installed resource types don't exist yet (but are referenced in the app manifest). If I install the CRD manually first, then deployment succeeds and GC works as expected when the application is deleted (the operator and everything it created is deleted). The CRD is the only thing which isn't GC'ed.

@RJPercival
Copy link

How is it that operators are currently supported (e.g. https://github.com/GoogleCloudPlatform/click-to-deploy/tree/master/k8s/spark-operator)?

@huyhg
Copy link
Contributor Author

huyhg commented Feb 5, 2019

Kubernetes GC doesn't work for non-namespaced objects, or objects outside of the owner's namespace. Thus, it's not possible to expect deletion of an Application object to trigger GC of a CRD.

The deployer installs the Spark operator deployment, giving it a service account with proper permissions. This deployment manages the Spark CRD. The deployment would need some deletion hook to remove the CRD. Note that, deleting the CRD will also delete all installed Spark clusters, so it's safer to leave the CRD behind.

Note that this paradigm works for a very specific case of operator: the marketplace application is the operator itself, not the instances that the operator spins up.

RJPercival pushed a commit to RJPercival/click-to-deploy that referenced this issue Feb 21, 2019
Remove EtcdCluster from Trillian app.yaml
4fac5e9 
The deployer does not wait for etcd-operator to install the CRD for
EtcdCluster before trying to use it, and so fails. Unfortunately, by
removing EtcdCluster from componentKinds, ownership information will not
be setup on the EtcdCluster and so it won't be uninstalled along with
Trillian.

GoogleCloudPlatform/marketplace-k8s-app-tools#303
tracks a feature request for supporting Kubernetes operators that install
a CRD.
RJPercival pushed a commit to RJPercival/click-to-deploy that referenced this issue Feb 25, 2019
Remove EtcdCluster from Trillian app.yaml
3169789 
The deployer does not wait for etcd-operator to install the CRD for
EtcdCluster before trying to use it, and so fails. Unfortunately, by
removing EtcdCluster from componentKinds, ownership information will not
be setup on the EtcdCluster and so it won't be uninstalled along with
Trillian.

GoogleCloudPlatform/marketplace-k8s-app-tools#303
tracks a feature request for supporting Kubernetes operators that install
a CRD.
RJPercival pushed a commit to RJPercival/click-to-deploy that referenced this issue Feb 25, 2019
Remove EtcdCluster from Trillian app.yaml
d91c6d8 
The deployer does not wait for etcd-operator to install the CRD for
EtcdCluster before trying to use it, and so fails. Unfortunately, by
removing EtcdCluster from componentKinds, ownership information will not
be setup on the EtcdCluster and so it won't be uninstalled along with
Trillian.

GoogleCloudPlatform/marketplace-k8s-app-tools#303
tracks a feature request for supporting Kubernetes operators that install
a CRD.
@RJPercival RJPercival mentioned this issue Feb 25, 2019
Closed
@RJPercival
Copy link

Interestingly, Helm have a similar problem and don't appear to have fixed it yet for operators. They've added a "crd-install" hook for installing CRDs before resources that depend on them, but that doesn't help much for operators that install the CRD themselves.

maelvls added a commit to jetstack/jetstack-secure-gcm that referenced this issue Feb 3, 2021
@maelvls
cannot use Helm 3's crds/ folder mechanism
53b7508 
As hinted in [1], it seems to be not possible (after multiple trials) to
manage CRDs using Helm 3's crd-install hook using the crds/ folder [2]. It
seems like the only way would be to bundle crds inside the templates/ like
cert-manager has been doing for a while.

About bundling CRDs inside templates/, Rob Percival mentions in [1] that
Helm has a problem with the CRD ordering [3] and that the issue has not
been fixed yet, which means installing operators like google-cas-issuer
breaks when the CRDs are inside templates/.

[1]: GoogleCloudPlatform/marketplace-k8s-app-tools#303
[2]: https://helm.sh/docs/developing_charts/#defining-a-crd-with-the-crd-install-hook
[3]: helm/helm#2994

Signed-off-by: Maël Valais <mael@vls.dev>
maelvls added a commit to jetstack/jetstack-secure-gcm that referenced this issue Feb 4, 2021
@maelvls
do not use Helm 3's crds/ folder that uses crd pre-install hook
9a4f70a 
As detailed in [1], CRDs that are applied through the CRD pre-install hook
will not ever be updated or upgraded. The Helm documentation reads [4]:

> The resources that a hook creates are not tracked or managed as part of
> the release. Once Tiller verifies that the hook has reached its ready
> state, it will leave the hook resource alone.
>
> Practically speaking, this means that if you create resources in a hook,
> you cannot rely upon helm delete to remove the resources. To destroy such
> resources, you need to either write code to perform this operation in a
> pre-delete or post-delete hook or add "helm.sh/hook-delete-policy"
> annotation to the hook template file.

On top of that, it seems to be not possible (after multiple trials) to
manage CRDs using Helm 3's crd-install hook using the crds/ folder [2]. It
seems like the only way would be to bundle crds inside the templates/ like
cert-manager has been doing for a while.

Note that installing CRDs using the templates/ way also causes trouble. Rob
Percival mentions in [1] that Helm has a problem with the CRD ordering [3]
and that the issue has not been fixed yet, which means installing operators
like google-cas-issuer breaks when the CRDs are inside templates/.

[1]: GoogleCloudPlatform/marketplace-k8s-app-tools#303
[2]: https://helm.sh/docs/developing_charts/#defining-a-crd-with-the-crd-install-hook
[3]: helm/helm#2994
[4]: https://v2.helm.sh/docs/developing_charts/#hook-resources-are-not-managed-with-corresponding-releases

Signed-off-by: Maël Valais <mael@vls.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
FR Feature Request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@RJPercival @huyhg