-
Notifications
You must be signed in to change notification settings - Fork 126
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
feat: add annotation to select container for version extraction #2471
Conversation
✅ Deploy Preview for keptn-lifecycle-toolkit ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
If a pod contains multiple containers the user can select which container the version should be extracted from by setting the keptn.sh/container annotation. If the annotation is set, but no container matches its name we assume something is wrong an throw an error. Signed-off-by: Norman <[email protected]>
239d451
to
0f3624c
Compare
Any advice on how to avoid the duplication and share the |
Hello, thank you for your contribution! Unfortunatelly, that's not possible, for now please duplicate it |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #2471 +/- ##
==========================================
- Coverage 85.77% 85.64% -0.13%
==========================================
Files 161 170 +9
Lines 10202 10528 +326
==========================================
+ Hits 8751 9017 +266
- Misses 1181 1231 +50
- Partials 270 280 +10
... and 16 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. |
factor out the actual tag extraction logic form calculateVersion to reduce duplication Signed-off-by: Norman <[email protected]>
Signed-off-by: Norman <[email protected]>
lifecycle-operator/webhooks/pod_mutator/handlers/pod_annotation.go
Outdated
Show resolved
Hide resolved
Signed-off-by: Norman <[email protected]>
@norman-zon thank you for your contribution! Would it be possible to introduce a full e2e test for this new feature? You can find lot's of examples already here. We are using kuttl for executing them. You can find more info in the official documentation |
I created a follow up ticket for the documentation updates: #2484 |
Signed-off-by: Norman <[email protected]>
SonarCloud Quality Gate failed. 0 Bugs No Coverage information Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Sorry, @odubajDT I don't have the capacities right now to get into e2e testing. Could someone else please take upon this task? |
Sure, I can follow-up on that and create the e2e tests. Do you have the capacity to add the 2 requested use-cases in the unit tests ? |
Thanks for taking over, @odubajDT. |
Thank you! |
@odubajDT should I squash into a single commit for more clarity? |
@norman-zon no need, the commits will be automatically squashed during merging, but thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @norman-zon for your contribution! LGTM ✅
I left only a minor note that we might need to reconsider in the future, but nothing for this PR :)
Description
If a pod contains multiple containers the user can select which container the version should be extracted from by setting the keptn.sh/container annotation. If the annotation is set, but no container matches its name we assume something is wrong an throw an error.
Part-of #2464
How to test
The tests
Test_calculateVersion
were expanded to cover the added functionalityChecklist
into multiple PRs)
see Contribution Guide
the Contribution Guide