feat: add support for annotations in func.yaml #314
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
What is the reason behind this? So you'd like to define knative annotations in another func.yaml property? Something like |
|
@zroubalik because when you try and set the scaling annotations manually, you get an error from Knative (or I am doing something wrong). Error: knative deployer failed to deploy the service: admission webhook "validation.webhook.serving.knative.dev" denied the request: validation failed: invalid key name "autoscaling.knative.dev/target": metadata.annotations
autoscaling annotations must be put under "spec.template.metadata.annotations" to work
invalid key name "autoscaling.knative.dev/window": metadata.annotations
autoscaling annotations must be put under "spec.template.metadata.annotations" to work
Error: exit status 1 |
lance
force-pushed
the
lance/307-add-annotations
branch
from
1f4cae3 to
d3cb765
Compare
zroubalik
approved these changes
Apr 28, 2021
This commit adds limited support for annotations in the func.yaml config file. The feature is limited, because it's only additive. A user can add an annotation `foo: bar` in the config and deploy the function, successfully setting that annotation on the Service. However, if they subsequently remove `foo: bar` from the config file, it will _not_ be removed from the deployment. This is because it's not possible to know, from the set of annotations that currently exist on the deployment, which ones were set by us and which were not. So, removing any annotations that are not in func.yaml is unsafe. It may be possible to store in a hidden file somewhere all of the user-supplied annotations, allowing us to diff func.yaml with that file, but I'm not sure I want to go down that path. It might just be best to document this limitation. We may also want to document that annotations added through func.yaml should be user supplied settings/values, and not annotations that are managed by knative (e.g. the autoscaling annotations). Fixes: knative#307 Signed-off-by: Lance Ball <lball@redhat.com>
lance
force-pushed
the
lance/307-add-annotations
branch
from
d3cb765 to
591f9a8
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit adds limited support for annotations in the func.yaml
config file. The feature is limited, because it's only additive. A
user can add an annotation
foo: barin the config and deploy thefunction, successfully setting that annotation on the Service.
However, if they subsequently remove
foo: barfrom the configfile, it will not be removed from the deployment. This is because
it's not possible to know, from the set of annotations that currently
exist on the deployment, which ones were set by us and which were not.
So, removing any annotations that are not in func.yaml is unsafe.
It may be possible to store in a hidden file somewhere all of the
user-supplied annotations, allowing us to diff func.yaml with that file,
but I'm not sure I want to go down that path. It might just be best to
document this limitation.
We may also want to document that annotations added through func.yaml
should be user supplied settings/values, and not annotations that are
managed by knative (e.g. the autoscaling annotations).
Fixes: #307
Signed-off-by: Lance Ball lball@redhat.com