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.

Sign up for GitHub

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

Jump to bottom

Gherkin for Alert details page- ODC-5485 #8132

Merged
merged 1 commit into from
Feb 19, 2021
Merged

Gherkin for Alert details page- ODC-5485 #8132

merged 1 commit into from
Feb 19, 2021

Conversation

sanketpathak
Copy link
Contributor

@openshift-ci-robot openshift-ci-robot added the component/dev-console Related to dev-console label Feb 10, 2021
gajanan-more
gajanan-more suggested changes Feb 11, 2021
View reviewed changes
Copy link
Contributor

@gajanan-more gajanan-more left a comment

Choose a reason for hiding this comment

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

Can you please update the file names with less characters and something meaningful?

.../dev-console/integration-tests/testData/monitoring/monitoring-cluster-monitoring-config.yaml Outdated
namespace: openshift-monitoring
data:
config.yaml: |
enableUserWorkload: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Need to add new line

Copy link
Contributor

Choose a reason for hiding this comment

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

We can add this YAMLs into testData. The PR with YAMLs folder is ready to merge. Once the PR is merged, you can add these YAMLs to YAMLs folder in testData.

frontend/packages/dev-console/integration-tests/features/monitoring/filter-the-alerts.feature
@@ -7,8 +7,8 @@ Feature: Filter for Alert state and Severity
Given user is at developer perspective
And user is at Add page
And user has created or selected namespace "aut-monitoring-alerts"
# To configure the alerts on cluster - need to execute below yaml file
# https://gist.githubusercontent.com/vikram-raj/27fa5c9d7e0cf223919c697d34bd2beb/raw/f85ab7a0e1f3e8a6270c124a4a97df13e5b9cb3c/ns-alert-setup.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

We can add this YAMLs into testData. The PR with YAMLs folder is ready to merge. Once the PR is merged, you can add these YAMLs to YAMLs folder in testData.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

...ckages/dev-console/integration-tests/testData/monitoring/monitoring-prometheous-example.yaml Outdated
expr: version{job="prometheus-example-app"}
- alert: VersionAlert
expr: version{job="prometheus-example-app"} == 0
for: 1s
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

...onsole/integration-tests/testData/monitoring/monitoring-user-workload-monitoring-config.yaml Outdated
data:
config.yaml: |
prometheusUserWorkload:
replicas: 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above

@andrewballantyne
Copy link
Contributor

/cc @vikram-raj
/uncc @andrewballantyne

@openshift-ci-robot openshift-ci-robot requested review from vikram-raj and removed request for andrewballantyne February 11, 2021 21:17
vikram-raj
vikram-raj reviewed Feb 12, 2021
View reviewed changes
frontend/packages/dev-console/integration-tests/features/monitoring/filter-the-alerts.feature Outdated
# To configure the alerts on cluster - need to execute below yaml file
# https://gist.githubusercontent.com/vikram-raj/27fa5c9d7e0cf223919c697d34bd2beb/raw/f85ab7a0e1f3e8a6270c124a4a97df13e5b9cb3c/ns-alert-setup.yaml
# To configure the alerts on cluster - need to execute below yaml files from monitoring YAMLs test data
# cluster-monitoring-config.yaml, prometheous-example.yaml, workload-monitoring-config.yaml
Copy link
Member

Choose a reason for hiding this comment

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

Change the order as we first need to apply the config.

Suggested change
# cluster-monitoring-config.yaml, prometheous-example.yaml, workload-monitoring-config.yaml
# cluster-monitoring-config.yaml, workload-monitoring-config.yaml, prometheous-example.yaml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

vikram-raj
vikram-raj reviewed Feb 12, 2021
View reviewed changes
...nd/packages/dev-console/integration-tests/testData/YAMLs/monitoring/prometheous-example.yaml Outdated
expr: version{job="prometheus-example-app"}
- alert: VersionAlert
expr: version{job="prometheus-example-app"} == 0
for: 1s
Copy link
Member

Choose a reason for hiding this comment

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

Add severity label, so that we can test the severity scenario.

Suggested change
for: 1s
for: 1s
labels:
severity: critical

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

@sanketpathak sanketpathak force-pushed the Alert-details-page-ODC-5485 branch 3 times, most recently from f251d7b to d8eff07 Compare February 15, 2021 13:24
vikram-raj
vikram-raj reviewed Feb 15, 2021
View reviewed changes
frontend/packages/dev-console/integration-tests/features/monitoring/monitoring-alerts.feature Outdated
Scenario: Silence alert from Alert details page
Given user is on Alert details page
When user clicks on Silence alert button
And user update the etails and click on Silence
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
And user update the etails and click on Silence
And user update the details and click on Silence

frontend/packages/dev-console/integration-tests/features/monitoring/monitoring-alerts.feature
Comment on lines +80 to +86
@regression
Scenario: Alert details page
Given user is on Alerts tab
When user clicks on the name of the alert
Then user will see Alert details page
And user will see alert Metrics
And user will see Name, Severity, Labels, Source, State, Alerting rule
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this regression is similar to this
https://github.com/openshift/console/pull/8132/files#diff-300a611e03ff2b74764a0d4d1c10ee3e6dc7e0726afb299ab5cd2605e3c686fbR152?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, this is from the Alert tab in the monitoring and that one you mentioned is from the Alert rule details page

Copy link
Member

Choose a reason for hiding this comment

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

ah got it. Thanks

vikram-raj
vikram-raj reviewed Feb 16, 2021
View reviewed changes
Copy link
Member

@vikram-raj vikram-raj left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 16, 2021
@sanketpathak
Copy link
Contributor Author

/assign @makambalaji

makambalaji
makambalaji reviewed Feb 17, 2021
View reviewed changes
...kages/dev-console/integration-tests/testData/YAMLs/monitoring/cluster-monitoring-config.yaml Outdated
namespace: openshift-monitoring
data:
config.yaml: |
enableUserWorkload: true
Copy link
Contributor

Choose a reason for hiding this comment

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

add empty line at the EOF

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other YAML files do not have an empty line as well so I didn't add in any files. Do you want me to add in these files

...nd/packages/dev-console/integration-tests/testData/YAMLs/monitoring/prometheous-example.yaml Outdated
expr: version{job="prometheus-example-app"} == 0
for: 1s
labels:
severity: critical
Copy link
Contributor

Choose a reason for hiding this comment

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

add Empty line at the EOF

...ages/dev-console/integration-tests/testData/YAMLs/monitoring/workload-monitoring-config.yaml Outdated
data:
config.yaml: |
prometheusUserWorkload:
replicas: 1
Copy link
Contributor

Choose a reason for hiding this comment

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

add empty line at the EOF

@openshift-ci-robot openshift-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lgtm Indicates that a PR is ready to be merged. labels Feb 19, 2021
@openshift-ci-robot openshift-ci-robot added component/pipelines Related to pipelines-plugin and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Feb 19, 2021
@makambalaji
Copy link
Contributor

/lgtm
/approve

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 19, 2021
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: makambalaji, sanketpathak, vikram-raj

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 19, 2021
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

4 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 099bf85 into openshift:master Feb 19, 2021
@spadgett spadgett added this to the v4.8 milestone Mar 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vikram-raj vikram-raj vikram-raj left review comments

@makambalaji makambalaji makambalaji left review comments

@gajanan-more gajanan-more gajanan-more requested changes

Assignees

@vikram-raj vikram-raj

@makambalaji makambalaji

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. component/dev-console Related to dev-console component/pipelines Related to pipelines-plugin lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
v4.8
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@sanketpathak @andrewballantyne @makambalaji @openshift-ci-robot @openshift-bot @vikram-raj @gajanan-more @spadgett @openshift-merge-robot