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

Allow custom handling of config-default. #8683

Closed
Cynocracy opened this issue Jul 17, 2020 · 7 comments
Closed

Allow custom handling of config-default. #8683

Cynocracy opened this issue Jul 17, 2020 · 7 comments
Labels
area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.

Comments

@Cynocracy
Copy link
Contributor

Describe the feature

As a platform operator, I would like to change the behavior of my flavor of Knative such that the 'default default' (or the behavior of config-default when a key is not present, defined here:

DefaultMaxRevisionTimeoutSeconds = 10 * 60
) is custom to my installation.

This would be useful for platform operators which are specializing, and may want to as an example enforce very short or very long deadlines by default.

@Cynocracy Cynocracy added the kind/feature Well-understood/specified features, ready for coding. label Jul 17, 2020
@knative-prow-robot knative-prow-robot added the area/API API objects and controllers label Jul 17, 2020
@Cynocracy
Copy link
Contributor Author

The simplest fix for this is just to make these flags.

Any objections?

@julz
Copy link
Member

julz commented Jul 18, 2020

I didn't quite follow why such an operator wouldn't want to apply those customisations to their config-defaults configmap directly?

@Cynocracy
Copy link
Contributor Author

Ah, there's a quirk in our current platform where we don't have the ability to set a value and allow it to be trivially user controlled in a configmap.

Thus, we need the 'value not present' default to match our desired system default.

@markusthoemmes
Copy link
Contributor

That seems to be counter-intuitive regarding the reason to have config-default in the first place IMO. We'd have a hard time explaining why we have both flags AND a config map expressing essentially the very same thing.

@mattmoor
Copy link
Member

I think what Jon's driving at is that there's a nuanced distinction between the cluster operator and the platform provider, and the configmap is generally targeting the former, but the latter may want to adjust things themselves.

Flags is one way of doing this, another would be to change these to var and use linker flags or the ./cmd entrypoint to change default values in-context.

@julz
Copy link
Member

julz commented Jul 20, 2020

Have to admit, I'm struggling to see the use case where you're willing to pass different flags (so modify images/yaml), but can't use kustomize (or similar) to modify the config map. If what we want is an ability to have different sets of defaults for different users (e.g. per-namespace defaults), that seems like a use case for #8114 (which I'm a big fan of.. maybe we should bump that up the priority list).

@github-actions
Copy link

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.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/API API objects and controllers kind/feature Well-understood/specified features, ready for coding. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale.
Projects
None yet
Development

No branches or pull requests

5 participants