add annotation to container spec and add validation to it #661
Conversation
Signed-off-by: Stephanie <yangcao@redhat.com>
yangcao77
requested review from
amisevsk,
davidfestal and
sleshchenko
as code owners
| // +optional | ||
| // Annotations to be added to Kubernetes Ingress or Openshift Route | ||
| Ingress map[string]string `json:"ingress,omitempty" patchStrategy:"merge" patchMergeKey:"name"` |
There was a problem hiding this comment.
Side question: are we okay with requiring all services/ingresses to share the same set of annotations?
There was a problem hiding this comment.
good point. I pushed a new commit to move the ingress/route annotation to be set under endpoint. so each endpoint can set it's own annotation.
for Odo, endpoints in a pod share the same service, unless dedicatedPod = true. Any usecase in che will have multiple services for containers within same pod?
There was a problem hiding this comment.
Any usecase in che will have multiple services for containers within same pod?
Currently no, but the spec doesn't require one service, so it could cause issues down the line. I think it still makes sense to keep service annotations workspace wide for now, though.
| Deployment map[string]string `json:"deployment,omitempty" patchStrategy:"merge" patchMergeKey:"name"` | ||
|
|
||
| // +optional | ||
| // Annotations to be added to service |
There was a problem hiding this comment.
We should pluralize service (and ingress/route below), since there may be multiple endpoints these annotations apply to.
| // Annotations to be added to service | |
| // Annotations to be added to services |
There was a problem hiding this comment.
same question as above, any usecases in che that will have multiple services for containers within same pod?
Signed-off-by: Stephanie <yangcao@redhat.com>
yangcao77
force-pushed
the
631-add-annotation-to-container
branch
from
2ccdb1d
to
4db5736
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: amisevsk, yangcao77 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Signed-off-by: Stephanie yangcao@redhat.com
What does this PR do?:
This PR adds annotation to container spec, and also add validation to it.
details see #632 (comment)
note that we've discussed to combine
ingressandroute(annotations) into one single term, and to be more generic, I chose to useingresssince it's a generic Kube term. and in the description it clarifies that the field is for both ingress annotations and route annotations.Which issue(s) this PR fixes:
#632
PR acceptance criteria:
Testing and documentation do not need to be complete in order for this PR to be approved. We just need to ensure tracking issues are opened.
Unit/Functional tests
QE Integration test
Documentation
Client Impact
How to test changes / Special notes to the reviewer: