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 step and aggregation fields for kubectl get KeptnMetric #2142

Closed
wants to merge 2 commits into from

Conversation

AryanSharma9917
Copy link
Contributor

This PR closes : #2100

@netlify
Copy link

netlify bot commented Sep 20, 2023

Deploy Preview for keptn-lifecycle-toolkit ready!

Name Link
🔨 Latest commit 7997ada
🔍 Latest deploy log https://app.netlify.com/sites/keptn-lifecycle-toolkit/deploys/650b598458369d0009acb21a
😎 Deploy Preview https://deploy-preview-2142--keptn-lifecycle-toolkit.netlify.app
📱 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.

@rakshitgondwal
Copy link
Member

You need to sign off your commits @AryanSharma9917

@rakshitgondwal rakshitgondwal changed the title Added Step and Aggregation fields for Kubectl get KeptnMetric feat: add step and aggregation fields for kubectl get KeptnMetric Sep 20, 2023
@codecov
Copy link

codecov bot commented Sep 20, 2023

Codecov Report

Merging #2142 (0d3c5b6) into main (bcc64e8) will increase coverage by 0.16%.
Report is 49 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2142      +/-   ##
==========================================
+ Coverage   84.34%   84.51%   +0.16%     
==========================================
  Files         151      154       +3     
  Lines        9601     9918     +317     
==========================================
+ Hits         8098     8382     +284     
- Misses       1218     1249      +31     
- Partials      285      287       +2     
Files Coverage Δ
metrics-operator/api/v1alpha3/keptnmetric_types.go 100.00% <ø> (ø)

... and 32 files with indirect coverage changes

Flag Coverage Δ
certificate-operator 65.91% <ø> (-2.64%) ⬇️
component-tests 57.82% <ø> (-1.07%) ⬇️
lifecycle-operator 85.02% <ø> (+0.08%) ⬆️
metrics-operator 87.38% <ø> (+0.82%) ⬆️
scheduler 32.12% <ø> (ø)

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

@AryanSharma9917
Copy link
Contributor Author

You need to sign off your commits @AryanSharma9917

Hi @rakshitgondwal, I have signed off my commit could you please review my PR

rakshitgondwal
rakshitgondwal previously approved these changes Sep 21, 2023
Copy link
Member

@rakshitgondwal rakshitgondwal left a comment

Choose a reason for hiding this comment

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

Just tested out the changes, and everything LGTM.
Screenshot 2023-09-21 200902

Thank you for your contribution @AryanSharma9917

@odubajDT
Copy link
Contributor

Thank you for your contribution! Can you also please update the CRD in our helm charts? Currently we need to update them manually https://github.com/keptn/lifecycle-toolkit/blob/main/helm/chart/templates/keptnmetric-crd.yaml#L191

Thank you!

@sonarqubecloud
Copy link

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

@odubajDT
Copy link
Contributor

odubajDT commented Sep 29, 2023

Please adapt the helm test results as well, execute:

helm template --namespace helmtests -f ./.github/scripts/.helm-tests/default/values.yaml ./helm/chart > ./.github/scripts/.helm-tests/default/result.yaml

from the root folder

Also please sign off your commits

@odubajDT
Copy link
Contributor

odubajDT commented Oct 6, 2023

Hello @AryanSharma9917 can you please resolve the conflicts against main branch ?

Thank you!

@rakshitgondwal
Copy link
Member

Hey @AryanSharma9917, are you still working on this?

@AryanSharma9917
Copy link
Contributor Author

Hey @AryanSharma9917, are you still working on this?

Yes @rakshitgondwal, I'll solve the issue ASAP..!

@rakshitgondwal
Copy link
Member

Hey @AryanSharma9917, any updates on this? if you are facing any issues let me know.

@odubajDT
Copy link
Contributor

Hey @AryanSharma9917 any updates on this PR? Thank you!

Copy link
Member

@mowies mowies left a comment

Choose a reason for hiding this comment

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

@AryanSharma9917 we moved the metrics API to v1beta1. It should be pretty easy to just move your changes over to the beta files. Then this PR can be merged!

@AryanSharma9917
Copy link
Contributor Author

Hi @mowies , @rakshitgondwal I have made the updated changes in this new PR could you please give it a look.
#2556

@odubajDT
Copy link
Contributor

Closing in favor of #2556

@odubajDT odubajDT closed this Nov 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add step and aggregation fields for kubectl get KeptnMetric
4 participants