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

feat: add configurable service account to KeptnTasks #2254

Merged
merged 21 commits into from
Nov 2, 2023

Conversation

prakrit55
Copy link
Member

@prakrit55 prakrit55 commented Oct 9, 2023

fixes:#1869

  • added spec.serviceAccount.name
  • use service account to generate jobs
  • add integration test for the field

@prakrit55 prakrit55 requested review from a team as code owners October 9, 2023 14:14
@netlify
Copy link

netlify bot commented Oct 9, 2023

Deploy Preview for keptn-lifecycle-toolkit ready!

Name Link
🔨 Latest commit 26466fc
🔍 Latest deploy log https://app.netlify.com/sites/keptn-lifecycle-toolkit/deploys/65439bc435b2840008c04a9a
😎 Deploy Preview https://deploy-preview-2254--keptn-lifecycle-toolkit.netlify.app/docs/crd-ref/lifecycle/v1alpha3
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added documentation Improvements or additions to documentation lifecycle-operator labels Oct 9, 2023
@codecov
Copy link

codecov bot commented Oct 11, 2023

Codecov Report

Merging #2254 (26466fc) into main (66668f5) will decrease coverage by 0.02%.
Report is 9 commits behind head on main.
The diff coverage is 76.92%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2254      +/-   ##
==========================================
- Coverage   85.50%   85.48%   -0.02%     
==========================================
  Files         161      161              
  Lines       10166    10165       -1     
==========================================
- Hits         8692     8690       -2     
- Misses       1196     1197       +1     
  Partials      278      278              
Files Coverage Δ
...rator/controllers/lifecycle/keptntask/job_utils.go 65.25% <100.00%> (+0.59%) ⬆️
...is/lifecycle/v1alpha3/keptntaskdefinition_types.go 75.00% <70.00%> (-25.00%) ⬇️

... and 6 files with indirect coverage changes

Flag Coverage Δ
certificate-operator 67.43% <ø> (ø)
component-tests 56.73% <100.00%> (-0.62%) ⬇️
lifecycle-operator 85.56% <76.92%> (-0.05%) ⬇️
metrics-operator 87.58% <ø> (ø)
scheduler 32.12% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@prakrit55
Copy link
Member Author

hey @RealAnna, could you once review it so that I am able to know what subsequent changes require here ??

@RealAnna
Copy link
Contributor

RealAnna commented Oct 11, 2023

@prakrit55 we have some helm tests based on the helm chart, you will have to fix them to add the new value in the crd

@prakrit55
Copy link
Member Author

and check here too
cc @mowies @RealAnna @bacherfl

@prakrit55
Copy link
Member Author

hey @mowies , @odubajDT can you chek it once again, if it needs anymore stuffs to work on

prakrit55 and others added 11 commits November 2, 2023 15:18
removed log line

Signed-off-by: Prakriti Mandal <98270250+prakrit55@users.noreply.github.com>
Signed-off-by: Griffin <prakritimandal611@gmail.com>
Signed-off-by: Griffin <prakritimandal611@gmail.com>
Signed-off-by: Griffin <prakritimandal611@gmail.com>
Signed-off-by: Griffin <prakritimandal611@gmail.com>
Signed-off-by: Griffin <prakritimandal611@gmail.com>
Signed-off-by: Griffin <prakritimandal611@gmail.com>
configured test

Signed-off-by: Prakriti Mandal <98270250+prakrit55@users.noreply.github.com>
updated job_utils_test.go

Signed-off-by: Prakriti Mandal <98270250+prakrit55@users.noreply.github.com>
updated test parameters

Signed-off-by: Prakriti Mandal <98270250+prakrit55@users.noreply.github.com>
Signed-off-by: Griffin <prakritimandal611@gmail.com>
@prakrit55 prakrit55 force-pushed the feature/serviceaccount-in-job branch from 377ca46 to 97ed2c4 Compare November 2, 2023 09:48
@prakrit55
Copy link
Member Author

Hey @prakrit55, now you can rebase to main and the test should be fixed

done 👍

odubajDT
odubajDT previously approved these changes Nov 2, 2023
@odubajDT
Copy link
Contributor

odubajDT commented Nov 2, 2023

Nice job! :)

Copy link
Member

@bacherfl bacherfl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good already, thank you for that contribution :) I just left some suggestions to make the documentation a bit tidier, but once those have been incorporated and the auto generated docs have been regenerated I think this is ready to get merged!

modified comments

Signed-off-by: Prakriti Mandal <98270250+prakrit55@users.noreply.github.com>
Signed-off-by: Griffin <prakritimandal611@gmail.com>
Copy link

sonarcloud bot commented Nov 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@bacherfl bacherfl merged commit e7db66f into keptn:main Nov 2, 2023
46 of 47 checks passed
@prakrit55 prakrit55 deleted the feature/serviceaccount-in-job branch November 30, 2023 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation lifecycle-operator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants