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

Fix flakiness in waitForStorageProfileMetricInit #3038

Merged
merged 2 commits into from
Dec 25, 2023

Conversation

arnongilboa
Copy link
Collaborator

@arnongilboa arnongilboa commented Dec 19, 2023

What this PR does / why we need it:
If CDI CR was deleted and re-created, the metric may have old instances of the old cdi-deployment pod IP.
Also added alerts Polarion test_ids and restore ginkgo output verbosity.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Release note:

NONE

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. size/XS labels Dec 19, 2023
@arnongilboa
Copy link
Collaborator Author

/retest

@akalenyu
Copy link
Collaborator

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akalenyu

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

@kubevirt-bot kubevirt-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2023
@akalenyu
Copy link
Collaborator

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2023
@arnongilboa
Copy link
Collaborator Author

/hold

@kubevirt-bot kubevirt-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/S and removed lgtm Indicates that a PR is ready to be merged. size/XS labels Dec 20, 2023
@arnongilboa
Copy link
Collaborator Author

/test pull-containerized-data-importer-e2e-destructive

If CDI CR was deleted and re-created, the metric may have old instances
of the old cdi-deployment pod IP.

Also added alerts Polarion test_ids and restore ginkgo output verbosity.

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
@arnongilboa arnongilboa changed the title Add alerts Polarion test_id Fix flakiness in waitForStorageProfileMetricInit Dec 24, 2023
@akalenyu
Copy link
Collaborator

/test pull-containerized-data-importer-e2e-destructive

@akalenyu
Copy link
Collaborator

/lgtm
/hold
couple of retests and I will cancel hold

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 24, 2023
@akalenyu
Copy link
Collaborator

/test pull-containerized-data-importer-e2e-ceph-wffc

@akalenyu
Copy link
Collaborator

/test pull-containerized-data-importer-e2e-destructive

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
@kubevirt-bot kubevirt-bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 24, 2023
@akalenyu
Copy link
Collaborator

/test pull-containerized-data-importer-non-csi-hpp
/test pull-containerized-data-importer-e2e-destructive
/hold cancel

@kubevirt-bot kubevirt-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 24, 2023
@akalenyu
Copy link
Collaborator

/hold
so doesn't merge before resolving conv

@kubevirt-bot kubevirt-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 24, 2023
@akalenyu
Copy link
Collaborator

/lgtm
/hold cancel
/test pull-containerized-data-importer-e2e-destructive

@kubevirt-bot kubevirt-bot added lgtm Indicates that a PR is ready to be merged. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels Dec 25, 2023
@kubevirt-bot kubevirt-bot merged commit 35979e8 into kubevirt:main Dec 25, 2023
18 checks passed
arnongilboa added a commit to arnongilboa/containerized-data-importer that referenced this pull request Dec 25, 2023
Manual backport of kubevirt#2998 & kubevirt#3038

- CDINoDefaultStorageClass - not having a default (or virt default)
SC is surely not an OpenShift error, as admins may prefer their cluster
users to only use explicit SC names. However, in the CDI context when
DV is created with default SC but default does not exist, we will fire
an error event and the PVC will be Pending for the default SC, so when
there are such Pending PVCs we will fire an alert.

- CDIDefaultStorageClassDegraded - when the default (or virt default)
SC does not support CSI/Snapshot clone (smart clone) or does not have
ReadWriteMany access mode (for live migration).

- CDIStorageProfilesIncomplete - add storageClass and provisioner
labels.

- CDIDataImportCronOutdated - add dataImportCron namespace and name
labels.

Also:
* Rename the metric kubevirt_cdi_storageprofile_status to
kubevirt_cdi_storageprofile_info since it always reports value 1,
where the label values provide the details about the storage class and
storage profile.
* Add snapshot manifests for tests and deploy snapshot CRDs in the hpp
destructive lane

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
arnongilboa added a commit to arnongilboa/containerized-data-importer that referenced this pull request Dec 25, 2023
Manual backport of kubevirt#2998 & kubevirt#3038

- CDINoDefaultStorageClass - not having a default (or virt default)
SC is surely not an OpenShift error, as admins may prefer their cluster
users to only use explicit SC names. However, in the CDI context when
DV is created with default SC but default does not exist, we will fire
an error event and the PVC will be Pending for the default SC, so when
there are such Pending PVCs we will fire an alert.

- CDIDefaultStorageClassDegraded - when the default (or virt default)
SC does not support CSI/Snapshot clone (smart clone) or does not have
ReadWriteMany access mode (for live migration).

- CDIStorageProfilesIncomplete - add storageClass and provisioner
labels.

- CDIDataImportCronOutdated - add dataImportCron namespace and name
labels.

Also:
* Rename the metric kubevirt_cdi_storageprofile_status to
kubevirt_cdi_storageprofile_info since it always reports value 1,
where the label values provide the details about the storage class and
storage profile.
* Add snapshot manifests for tests and deploy snapshot CRDs in the hpp
destructive lane

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
kubevirt-bot pushed a commit that referenced this pull request Dec 26, 2023
…3040)

Manual backport of #2998 & #3038

- CDINoDefaultStorageClass - not having a default (or virt default)
SC is surely not an OpenShift error, as admins may prefer their cluster
users to only use explicit SC names. However, in the CDI context when
DV is created with default SC but default does not exist, we will fire
an error event and the PVC will be Pending for the default SC, so when
there are such Pending PVCs we will fire an alert.

- CDIDefaultStorageClassDegraded - when the default (or virt default)
SC does not support CSI/Snapshot clone (smart clone) or does not have
ReadWriteMany access mode (for live migration).

- CDIStorageProfilesIncomplete - add storageClass and provisioner
labels.

- CDIDataImportCronOutdated - add dataImportCron namespace and name
labels.

Also:
* Rename the metric kubevirt_cdi_storageprofile_status to
kubevirt_cdi_storageprofile_info since it always reports value 1,
where the label values provide the details about the storage class and
storage profile.
* Add snapshot manifests for tests and deploy snapshot CRDs in the hpp
destructive lane

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
arnongilboa added a commit to arnongilboa/containerized-data-importer that referenced this pull request Dec 26, 2023
Manual backport of kubevirt#2998 & kubevirt#3038

- CDINoDefaultStorageClass - not having a default SC is surely not an
OpenShift error, as admins may prefer their cluster users to only use
explicit SC names. However, in the CDI context when DV is created with
default SC but default does not exist, we will fire an error event and
the PVC will be Pending for the default SC, so when there are such
Pending PVCs we will fire an alert.

- CDIDefaultStorageClassDegraded - when the default SC does not support
CSI/Snapshot clone (smart clone) or does not have ReadWriteMany access
mode (for live migration).

- CDIStorageProfilesIncomplete - add storageClass and provisioner
labels.

- CDIDataImportCronOutdated - add dataImportCron namespace and name
labels.

Also:
* Rename the metric kubevirt_cdi_storageprofile_status to
kubevirt_cdi_storageprofile_info since it always reports value 1,
where the label values provide the details about the storage class and
storage profile.
* Add snapshot manifests for tests and deploy snapshot CRDs in the hpp
destructive lane

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
kubevirt-bot pushed a commit that referenced this pull request Jan 3, 2024
…3041)

Manual backport of #2998 & #3038

- CDINoDefaultStorageClass - not having a default SC is surely not an
OpenShift error, as admins may prefer their cluster users to only use
explicit SC names. However, in the CDI context when DV is created with
default SC but default does not exist, we will fire an error event and
the PVC will be Pending for the default SC, so when there are such
Pending PVCs we will fire an alert.

- CDIDefaultStorageClassDegraded - when the default SC does not support
CSI/Snapshot clone (smart clone) or does not have ReadWriteMany access
mode (for live migration).

- CDIStorageProfilesIncomplete - add storageClass and provisioner
labels.

- CDIDataImportCronOutdated - add dataImportCron namespace and name
labels.

Also:
* Rename the metric kubevirt_cdi_storageprofile_status to
kubevirt_cdi_storageprofile_info since it always reports value 1,
where the label values provide the details about the storage class and
storage profile.
* Add snapshot manifests for tests and deploy snapshot CRDs in the hpp
destructive lane

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
alromeros pushed a commit to alromeros/containerized-data-importer that referenced this pull request Jan 5, 2024
* Fix flakiness in waitForStorageProfileMetricInit

If CDI CR was deleted and re-created, the metric may have old instances
of the old cdi-deployment pod IP.

Also added alerts Polarion test_ids and restore ginkgo output verbosity.

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

* Fix pods_high_restart metrics to be 0 when no pods

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>

---------

Signed-off-by: Arnon Gilboa <agilboa@redhat.com>
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. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. lgtm Indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/M
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants