Why do we need kubernetes-sigs/application controller in Kubeflow?

[Bobgy]

When I was investigating issue: GoogleCloudPlatform/kubeflow-distribution#184,

I realized there's a huge volume of logs coming from application controller, and so far I think that's normal behavior, we need a way to change log level, but I don't think it exposed such a config.

However, a bigger question is why is Kubeflow using application controller? Does it solve anyone's problem? It seems https://github.com/kubernetes-sigs/application doesn't have much activity any more.

As a developer, I almost never look at application CRs, because I know the low-level details. There doesn't seem to be enough UI/guidance that teach entrance level people to use them.

This is what I feel about it, I'd like to know more context.

/cc @jlewi @kubeflow/wg-deployment-leads

[Bobgy]

I can see it's enforced through https://github.com/kubeflow/community/blob/master/guidelines/application_requirements.md, but I don't fully get the reasoning behind. Was kubernetes-sigs/application ambitious at that time, but they seem to fail to achieve that goal now?

[Bobgy]

Additionally, I think we've got many issues with application maintenance like #1573, no one before thesuperzapper noticed the problem seems to be an evidence that no one used it at all.

[Bobgy]

Sorry, I mentioned the wrong group.
I meant @kubeflow/wg-manifests-leads, not @kubeflow/wg-deployment-leads, but maybe it's worth both groups' discussion.

[yanniszark]

@Bobgy you ninja'd me there, I was planning to ask this question myself in the doc we're preparing for manifests best practices, as mentioned here: #1710

It could also lead to weird and hard-to-debug issues, like the one seen here:
#540
kubeflow/kubeflow#4767 (comment)

If adding cluster-wide resources by mistake to an Application CR can lead to resources being accidentally deleted, this is very worrying by itself.

As to the value it provides, it enables users to see and handle the high-level apps installed in the cluster. However, given the inactivity of the upstream repo (https://github.com/kubernetes-sigs/application) and the limitations it has (what about cluster-level resources?, excessive logging), I don't know if it makes sense to prescribe this everywhere.

[Bobgy]

Haha, good to know you have also thought of it.

[yanniszark]

I prepared a small overview of the application cr in Kubeflow.

What is it used for:

• The ability to describe an applications metadata (e.g., that an application like notebook-controller is running). This is why the GKE marketplace demands it.
• Tying all configuration (e.g., CRDs, Deployments, etc) relevant to an App together. It doesn't support cross-namespace references.

Limitations:

• It selects resources by labels and then adds ownerReferences to the Application CR (optional, but we have it turned on everywhere). This means it can't select resources in many namespaces OR cluster-level resources. Selecting a cluster-level resource can cause silent damage (Cross namespace ownership will cause resources to be GC'd; are we setting owner references for any cluster scoped resources? #540)
• The application-controller produces tons of logs, causing big bills for unexpecting users (Stackdriver Logs are very expensive for kubeflow GoogleCloudPlatform/kubeflow-distribution#184). Is this perhaps affecting us as well?
• It needs to be constantly updated with all the resource kinds that it must watch, instead of inferring them from the labels. This leads to it going out-of-sync (most Application Resource Selectors are Incorrect #1573).
• It hasn’t picked up much traction and the repo is light on activity.
  Because of these, the second point is pretty much nil. The Application CR fails to select and anchor all relevant config to it. The only remaining point is that it packages some metadata about an application.

Proposed Solution
The Application CR has failed to achieve its goals. It is an extra maintainance burden, so we should remove it.
@Bobgy what do you think? Perhaps we should have a quick slot one of these days to discuss further?

[Bobgy]

I agree with that, plus.. we may use kpt live status instead for the monitoring functionality.

[thesuperzapper]

@yanniszark agree with removal, but since its the responsibility of component owners to do this, we should do a quick run through before the 1.3 release to check everyone has removed them

[akdigitalself]

Thanks for giving a background on this issue. The other issues where this issue is referenced are resolved. The application manifest exists in the 1.3 branch. Is there a plan to remove it in v1.3.1?

[yanniszark]

@akdigitalself the application controller exists under contrib, which contains 3rd-party packages not supported by a Kubeflow WG. But yes, we should clean it up for 1.4. Would you like to open a PR for that?

[Bobgy]

Some installations need it, so we don't want to remove it yet. It's already not included in platform-agnostic

[juliusvonkohout]

Some installations need it, so we don't want to remove it yet. It's already not included in platform-agnostic

@Bobgy are you sure that you did not accidentally leave it in ?

manifests/apps/pipeline/upstream/env/platform-agnostic-multi-user/kustomization.yaml

Line 13 in 18b9bc8

- ../../third-party/metacontroller/base

[Bobgy]

@juliusvonkohout that's metacontroller, not application controller.
https://github.com/metacontroller/metacontroller is a dependency of KFP profile controller (for multi-user mode), but KFP profile controller is actually a pretty simple script, if you don't like the dependency you can totally just deploy KFP namespaced resources on your own or use a different controller.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in one week if no further activity occurs. Thank you for your contributions.

[stale]

This issue has been closed due to inactivity.