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

test: improve metrics integration tests by adding kuttl log collectors #2765

Merged
merged 21 commits into from
Jan 18, 2024
Merged

test: improve metrics integration tests by adding kuttl log collectors #2765

merged 21 commits into from
Jan 18, 2024

Conversation

rizul2108
Copy link
Contributor

I have added collectors for the Metrics Integration Tests.

Fixes #2699

@rizul2108 rizul2108 requested a review from a team as a code owner January 7, 2024 05:50
@rizul2108 rizul2108 changed the title add collectors for metrics tests test: improve metrics integration tests by adding kuttl log collectors Jan 7, 2024
Copy link

codecov bot commented Jan 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (71f20a6) 85.71% compared to head (29e4230) 85.62%.
Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2765      +/-   ##
==========================================
- Coverage   85.71%   85.62%   -0.10%     
==========================================
  Files         160      160              
  Lines       10163    10140      -23     
==========================================
- Hits         8711     8682      -29     
- Misses       1171     1176       +5     
- Partials      281      282       +1     

see 13 files with indirect coverage changes

Flag Coverage Δ
certificate-operator 67.42% <ø> (ø)
component-tests 56.18% <ø> (-1.50%) ⬇️
lifecycle-operator 84.62% <ø> (-0.11%) ⬇️
metrics-operator 87.65% <ø> (+<0.01%) ⬆️
scheduler 36.53% <ø> (ø)

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

Signed-off-by: Rizul Gupta <[email protected]>
@odubajDT
Copy link
Contributor

odubajDT commented Jan 8, 2024

hello @rizul2108 !

Can you please fit the yamllint errors?

here are the logs:

./test/testmetrics/metrics-hpa/01-assert.yaml
23:5 error no new line character at the end of file (new-line-at-end-of-file)
23:1 error trailing spaces (trailing-spaces)

./test/testmetrics/metrics/02-teststep-assert.yaml
17:5 error no new line character at the end of file (new-line-at-end-of-file)
17:1 error trailing spaces (trailing-spaces)

Thank you!

@RealAnna
Copy link
Contributor

RealAnna commented Jan 8, 2024

Hi @rizul2108 thank you for your contribution! As by the issue, you need to modify your command to also log

  1. metrics-operator pod
  2. KeptnMetric and KeptnMetricsProvider

rizul2108 and others added 2 commits January 9, 2024 21:45
@rizul2108
Copy link
Contributor Author

rizul2108 commented Jan 10, 2024

Hi @rizul2108 thank you for your contribution! As by the issue, you need to modify your command to also log

  1. metrics-operator pod
    Can you please guide a little where I have to add collectors for this pod as I am unable to find the yaml file containing code for this pod?
  2. KeptnMetric and KeptnMetricsProvider
    And I wanted to confirm for adding pod fo these 2 I have to make separate file named 00-assert.yaml in folder (https://github.com/keptn/lifecycle-toolkit/tree/main/test/testmetrics/metrics-provider ) or I can add collectors in already existing 00-install.yaml only ?

@RealAnna @odubajDT

@odubajDT
Copy link
Contributor

odubajDT commented Jan 11, 2024

Hi @rizul2108 thanks for reaching out

Hi @rizul2108 thank you for your contribution! As by the issue, you need to modify your command to also log

  1. metrics-operator pod
    Can you please guide a little where I have to add collectors for this pod as I am unable to find the yaml file containing code for this pod?

metrics-operator is a standard pod in the Keptn default namespace (keptn-system). You should be able to access its logs in the same manner as you are already doing with the pods of the podtato-head application

  1. KeptnMetric and KeptnMetricsProvider
    And I wanted to confirm for adding pod fo these 2 I have to make separate file named 00-assert.yaml in folder (https://github.com/keptn/lifecycle-toolkit/tree/main/test/testmetrics/metrics-provider ) or I can add collectors in already existing 00-install.yaml only ?

Please be aware, these are not pods, those are resources, you can execute kubectl describe on them

@RealAnna @odubajDT

@rizul2108
Copy link
Contributor Author

rizul2108 commented Jan 11, 2024

Hi @rizul2108 thanks for reaching out

Hi @rizul2108 thank you for your contribution! As by the issue, you need to modify your command to also log

  1. metrics-operator pod
    Can you please guide a little where I have to add collectors for this pod as I am unable to find the yaml file containing code for this pod?

metrics-operator is a standard pod in the Keptn default namespace (keptn-system). You should be able to access its logs in the same manner as you are already doing with the pods of the podtato-head application
So should I add command to get logs of metrics-operator pod to same 2 files where before I added?
@odubajDT

Signed-off-by: Rizul Gupta <[email protected]>
@odubajDT
Copy link
Contributor

Hi @rizul2108 thanks for reaching out

Hi @rizul2108 thank you for your contribution! As by the issue, you need to modify your command to also log

  1. metrics-operator pod
    Can you please guide a little where I have to add collectors for this pod as I am unable to find the yaml file containing code for this pod?

metrics-operator is a standard pod in the Keptn default namespace (keptn-system). You should be able to access its logs in the same manner as you are already doing with the pods of the podtato-head application
So should I add command to get logs of metrics-operator pod to same 2 files where before I added?
@odubajDT

You should get logs from the metrics-operator, for the rest should be enough to use kubectl describe

@rizul2108
Copy link
Contributor Author

Ok sure, I will change it and push that in some time.

@rizul2108
Copy link
Contributor Author

Can anyone of you please review it now? @RealAnna @odubajDT

@rizul2108
Copy link
Contributor Author

The sonar cloud test is failing what I will have to change to pass this test?

@odubajDT
Copy link
Contributor

odubajDT commented Jan 17, 2024

The sonar cloud test is failing what I will have to change to pass this test?

That's not caused by your code changes, you can ignore the failure

@rizul2108
Copy link
Contributor Author

The sonar cloud test is failing what I will have to change to pass this test?

That's not caused by your code changes, you can ignore the failure

Ok thanks @odubajDT

Signed-off-by: Rizul Gupta <[email protected]>
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions

3.5% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Copy link
Contributor

@odubajDT odubajDT left a comment

Choose a reason for hiding this comment

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

Thank you for contribution!

Copy link
Contributor

@RealAnna RealAnna left a comment

Choose a reason for hiding this comment

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

LGTM thank you for the copntribution!

@odubajDT odubajDT merged commit 728cea8 into keptn:main Jan 18, 2024
29 of 30 checks passed
Vickysomtee pushed a commit to Vickysomtee/keptn-lifecycle-toolkit that referenced this pull request Apr 23, 2024
keptn#2765)

Signed-off-by: Rizul Gupta <[email protected]>
Signed-off-by: Rizul Gupta <[email protected]>
Signed-off-by: odubajDT <[email protected]>
Co-authored-by: odubajDT <[email protected]>
Signed-off-by: vickysomtee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add collectors to Metrics Integration Tests
3 participants