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

Jump to bottom

Bug 1946232: VM List can consume very high CPU #9761

Merged
merged 1 commit into from Aug 13, 2021
Merged

Bug 1946232: VM List can consume very high CPU #9761

merged 1 commit into from Aug 13, 2021

Conversation

glekner
Copy link
Contributor

@glekner glekner commented Aug 9, 2021

blocked by #9758

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 9, 2021

@glekner: An error was encountered searching for bug 1946232 on the Bugzilla server at https://bugzilla.redhat.com. No known errors were detected, please see the full error message for details.

Full error message. could not unmarshal response body: invalid character '<' looking for beginning of value

Please contact an administrator to resolve this issue, then request a bug refresh with /bugzilla refresh.

In response to this:

Bug 1946232: VM List can consume very high CPU

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.

@openshift-ci openshift-ci bot requested review from gouyang and yaacov August 9, 2021 13:29
@openshift-ci openshift-ci bot added component/kubevirt Related to kubevirt-plugin approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Aug 9, 2021
@glekner glekner force-pushed the improve-vm-table-performance-2 branch from fb26b11 to 7272bab Compare August 9, 2021 13:30
@glekner
Copy link
Contributor Author

glekner commented Aug 9, 2021

/retest

@glekner glekner force-pushed the improve-vm-table-performance-2 branch from 7272bab to 4a20309 Compare August 9, 2021 15:23
@openshift-ci openshift-ci bot added the kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated label Aug 9, 2021
@yaacov
Copy link
Member

yaacov commented Aug 10, 2021

/retest

@yaacov
Copy link
Member

yaacov commented Aug 10, 2021

#9758 - merged

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 10, 2021
@yaacov
Copy link
Member

yaacov commented Aug 10, 2021

/retest

@openshift-bot
Copy link
Contributor

/retest-required

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

1 similar comment
@openshift-bot
Copy link
Contributor

/retest-required

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

@yaacov
Copy link
Member

yaacov commented Aug 10, 2021

/retest

@openshift-bot
Copy link
Contributor

/retest-required

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

3 similar comments
@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@openshift-bot
Copy link
Contributor

/retest-required

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

@glekner glekner force-pushed the improve-vm-table-performance-2 branch from 4a20309 to bf53977 Compare August 11, 2021 09:10
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Aug 11, 2021
@glekner glekner force-pushed the improve-vm-table-performance-2 branch 4 times, most recently from cae5b10 to 1150957 Compare August 11, 2021 11:21
@glekner
Copy link
Contributor Author

glekner commented Aug 11, 2021

/retest

@glekner
Copy link
Contributor Author

glekner commented Aug 11, 2021 •
edited

@zcahana hey, cc @yaacov
I'm noticing the printableStatus is Provisioning when the test is expecting it to be Stopped
the vm has no runStrategy, why is the status Provisioning?
image

@yaacov
Copy link
Member

yaacov commented Aug 12, 2021

/retest

p.s.
@glekner if the printableStatus show provisionining when spec.running=false we can address thins discrepancy in the conditions column by stating "(icon) Stopped" in the conditions column

@glekner glekner force-pushed the improve-vm-table-performance-2 branch 2 times, most recently from 1f6dd66 to f7c49e0 Compare August 12, 2021 07:36
@glekner
Copy link
Contributor Author

glekner commented Aug 12, 2021

/test e2e-gcp-console

@glekner
Copy link
Contributor Author

glekner commented Aug 12, 2021

/test kubevirt-plugin

@glekner
Copy link
Contributor Author

glekner commented Aug 12, 2021

/test e2e-gcp-console

@glekner glekner force-pushed the improve-vm-table-performance-2 branch from f7c49e0 to a84beda Compare August 12, 2021 18:10
@glekner
Copy link
Contributor Author

glekner commented Aug 12, 2021

/retest

@yaacov
Copy link
Member

yaacov commented Aug 13, 2021

/bugzilla refresh

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 13, 2021

@yaacov: This pull request references Bugzilla bug 1946232, 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.9.0) matches configured target release for branch (4.9.0)
  • bug is in the state NEW, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @gouyang

In response to this:

/bugzilla refresh

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.

@openshift-ci openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high 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 13, 2021
yaacov
yaacov reviewed Aug 13, 2021
View reviewed changes
frontend/packages/kubevirt-plugin/integration-tests-cypress/view/vm.ts
if (status === VM_STATUS.Running) {
const { name, namespace } = vmData;
cy.waitForLoginPrompt(name, namespace);
}

if (status === VM_STATUS.Starting) {
Copy link
Member

Choose a reason for hiding this comment

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

a. https://github.com/kubevirt/user-guide/pull/439/files
maybe we may report starting when real status is provisioning ?
we should look at the logic for reporting "starting" and see if in some cases we should report "provisioning" instead ?

b. can we make a new method
export const waitForStatuses = (statuses: string[], vmData?: VirtualMachineData, timeout?: number) => {
and use it when we can continue if we see one of the correct statuses ?
e.g.
continue to next step if vm status is in ["starting", "provisioning"] acceptable statuses list ...

@yaacov
Copy link
Member

yaacov commented Aug 13, 2021

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 13, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 13, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: glekner, yaacov

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

yaacov
yaacov reviewed Aug 13, 2021
View reviewed changes
frontend/packages/kubevirt-plugin/integration-tests-cypress/tests/vm/advanced-wizard.ts
@@ -87,7 +87,7 @@ describe('Test VM creation', () => {
});
});

[urlVM, registryVM, pvcVM, pxeVM].forEach((vmData) => {
[urlVM, registryVM, pvcVM].forEach((vmData) => {
Copy link
Member

Choose a reason for hiding this comment

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

NOTE:
this is skipping the PXE test, we will need to re-introduce this test once we fix the test flakynes
I don't think the PXE test flakynes is connected to this PR

@openshift-ci openshift-ci bot merged commit 253b444 into openshift:master Aug 13, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 13, 2021

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

Bugzilla bug 1946232 has been moved to the MODIFIED state.

In response to this:

Bug 1946232: VM List can consume very high CPU

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.

@zcahana
Copy link

zcahana commented Aug 15, 2021

I'm noticing the printableStatus is Provisioning when the test is expecting it to be Stopped
the vm has no runStrategy, why is the status Provisioning?

@glekner (cc @yaacov) sorry for the delayed response (was on vacation 😎).

This behavior is actually by design. The different VM states aren't mutually exclusive, some of them coexist (e.g., Stopped and Provisioning, or Running and Migrating). Since we include only one state in printableStatus, we choose the one we think is more significant. For this particular case, Provisioning was chosen over Stopped since the DataVolumes for the VM are currently being prepared (provisioned, populated with data, etc). Once this is done, the printableStatus field should change back to Stopped.

@spadgett spadgett added this to the v4.9 milestone Aug 30, 2021
@openshift-ci openshift-ci bot mentioned this pull request Oct 10, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yaacov yaacov yaacov left review comments

@gouyang gouyang Awaiting requested review from gouyang

Assignees

@yaacov yaacov

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. bugzilla/severity-high Referenced Bugzilla bug's severity is high 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/kubevirt Related to kubevirt-plugin kind/i18n Indicates issue or PR relates to internationalization or has content that needs to be translated lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Milestone
v4.9
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@glekner @yaacov @openshift-bot @zcahana @spadgett