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

Bug 1872644: gherkin script for HPA-add,edit an delete-ODC-3556 #6442

Merged

Conversation

sanketpathak
Copy link
Contributor

Gherkin script for the HPA -Add, Edit and Delete
Epic: https://issues.redhat.com/browse/ODC-3556
Story: https://issues.redhat.com/browse/ODC-4339

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.

You haven't added the file extension to any file. Can you please add it? Also create bugzilla and add it here.

@sanketpathak sanketpathak changed the title gherkin script for HPA-add,edit an delete-ODC-3556 Bug 1872644: gherkin script for HPA-add,edit an delete-ODC-3556 Aug 26, 2020
@openshift-ci-robot openshift-ci-robot added bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Aug 26, 2020
@openshift-ci-robot
Copy link
Contributor

@sanketpathak: This pull request references Bugzilla bug 1872644, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target release (4.6.0) matches configured target release for branch (4.6.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

In response to this:

Bug 1872644: gherkin script for HPA-add,edit an delete-ODC-3556

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@sanketpathak sanketpathak force-pushed the gherkin-hpa-ODC-3556 branch 3 times, most recently from ece1d12 to 3f7be81 Compare August 26, 2020 17:45
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.

Please update the suggested changes.

@sanketpathak sanketpathak force-pushed the gherkin-hpa-ODC-3556 branch 3 times, most recently from 20daa0e to 945001d Compare August 27, 2020 12:26
And user checks the YAML
And user changes values of cpu and memory under metrics.resource.target.averageUtilization
And user changes minimum and maximum pods value in minReplicas and maxReplicas
And user edits the name value under metadata.name
Copy link
Contributor

Choose a reason for hiding this comment

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

So you can't edit the name from Form view but you can edit it from YAML view, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can be edited but you can't save it as the name should not be changed

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you able to save the changed name through YAML? If yes, then don't you think it's a bug? Can you please confirm it from Andrew?

@sanketpathak sanketpathak force-pushed the gherkin-hpa-ODC-3556 branch 4 times, most recently from e13801c to 9c8c852 Compare August 28, 2020 09:56
@gajanan-more
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 28, 2020
Copy link
Contributor

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

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

You seem to (unless I missed it) not cover what if the user doesn't set the cpu / memory limits and the disabled form.

And user selects on resource tab
Then user can see two pods under pod section
And user can see Horizontal Pod Autoscalers section
And user can see the example with HPA tag associated present under HPA section
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
And user can see the example with HPA tag associated present under HPA section
And user can see the 'example' with HPA tag associated present under HPA section

Thoughts? example is a bad word imo for tests, because I read this as "an example" rather than "the 'example' named resource"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed

And user has created a deployment
And user has added CPU resource limit
And user has added Memory resource limit
And user is in topology
Copy link
Contributor

Choose a reason for hiding this comment

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

And user has created an HPA on this deployment?

None of the edit flows make sense without this pre-step

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've included this in the given statement of the scenarios

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that, but what's the purpose of a background if not to reduce repeat?

And user has a workload with HPA assigned to it

@regression @manual
Scenario: Changes due to HPA in Workload Sidebar
Copy link
Contributor

Choose a reason for hiding this comment

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

We should also check that there is also no set pod count in the actions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm checking for Edit Pod Count option in the action menu

Copy link
Contributor

Choose a reason for hiding this comment

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

Must have missed it.

And user clicks on details tab
Then user can see the scaling of the pod disabled
And user can see the arrows to increase and decrease pods are not present
And user can see 'Autoscaling to' inside the pod donut
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't tie to this text, it should settle when the HPA cleans up. And thus this may say 'Autoscaled to'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough I'll edit it

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Aug 28, 2020
Copy link
Contributor

@andrewballantyne andrewballantyne left a comment

Choose a reason for hiding this comment

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

/lgtm

You should get Balaji to give the approval.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 28, 2020
@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 28, 2020
Comment on lines 49 to 53
And user adds name 'example'
And user assigns minimum pods value to 2
And user assigns maximum pods value to 5
And user gives CPU Utilizationvalue as 60
And user gives Memory Utilization value as 30
Copy link
Contributor

Choose a reason for hiding this comment

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

Include the test data in ""

Copy link
Contributor

@makambalaji makambalaji left a comment

Choose a reason for hiding this comment

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

Scenarios looks good, make sure below acceptance criteria is added
As a user, when i create an HPA from the HPA page in the Admin perspective, I should be brought to the YAML editor with default YAML aligned with v2beta2
As a user, I want to Search for HPAs, click on the Create button and be brought to the YAML editor with default YAML aligned with v2beta2

Please note: Add test data wherever aplicable

Comment on lines 73 to 74
And user assigns name value as example
And user gives value to averageUtilization under cpu target as 60
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test data in ""


@regression
Scenario: Edit HPA action form view
Given user has a workload with HPA assigned to it
Copy link
Contributor

Choose a reason for hiding this comment

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

Include the test data wherever applicable i.e, workload as below
user has a workload "nodejs-ex-git-1" with HPA assigned to it

Comment on lines 22 to 23
And user checks and edit the cpu value
And user checks and edit the memory value
Copy link
Contributor

Choose a reason for hiding this comment

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

Add test data for cpu, memory values

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. and removed lgtm Indicates that a PR is ready to be merged. labels Sep 3, 2020
@makambalaji
Copy link
Contributor

/approve

@makambalaji
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 4, 2020
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne, gajanan-more, makambalaji, sanketpathak

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-merge-robot openshift-merge-robot merged commit 96ecdbb into openshift:master Sep 4, 2020
@openshift-ci-robot
Copy link
Contributor

@sanketpathak: All pull requests linked via external trackers have merged:

Bugzilla bug 1872644 has been moved to the MODIFIED state.

In response to this:

Bug 1872644: gherkin script for HPA-add,edit an delete-ODC-3556

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@spadgett spadgett added this to the v4.6 milestone Sep 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-low Referenced Bugzilla bug's severity is low for the branch this PR is targeting. bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. component/dev-console Related to dev-console lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants