search

knative / func

Knative Functions client API and CLI
Apache License 2.0
263 stars 135 forks source link

Explicit, implicit or auto configuring cluster domains #86

Open lkingland opened 3 years ago

lkingland commented 3 years ago

https://github.com/boson-project/faas/blob/fbab8c09d05e0f08c5320c0d16b9508203e458d1/knative/deployer.go#L61-L63

It is my understanding that the label and annotation referencing the domain and subdomain are not necessary for the system to function if the configuraiton of the cluster domains is followed per the docs. This ConfigMap configuration mutation could also be automated.

Assuming the implementation is fixed per #85 , keeping these lines has the benefit that the cluster configuration is not strictly required. It has the detriment that one can no longer use kn or other non-faas-cli methods for creating a Function service with a route unless they remember to manually add these labels/annotations.

I would probably lean towards expecting that the cluster is configured correctly (I presume we will have automatin to help with this soon) rather than rely on the client to set these values. In that case, these two lines could be removed, and the check in #85 could actually be removed entirely.

Both approaches (three, really) have differing tradeoffs, so thoughts much appreciated.

github-actions[bot] commented 2 years ago

This issue is stale because it has been open for 90 days with no activity. It will automatically close after 30 more days of inactivity. Reopen the issue with /reopen. Mark the issue as fresh by adding the comment /remove-lifecycle stale.