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

Fix test failure - missing pod following recreate of CDI #2744

Merged
merged 1 commit into from
Jun 27, 2023
Merged

Fix test failure - missing pod following recreate of CDI #2744

merged 1 commit into from
Jun 27, 2023

Conversation

akalenyu
Copy link
Collaborator

@akalenyu akalenyu commented Jun 7, 2023 •
edited

What this PR does / why we need it:
From some debugging, it looks like the priority class tests don't properly wait for
redeploy to finish and thus the following tests get an initial wrong slice of cdiPods
(Consider 2 cdi-uploadproxy pods, one of which is terminating, making way to the new one)

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
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@kubevirt-bot kubevirt-bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has DCO signed all their commits. labels Jun 7, 2023
@akalenyu
Copy link
Collaborator Author

akalenyu commented Jun 7, 2023

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

@akalenyu
Copy link
Collaborator Author

akalenyu commented Jun 7, 2023

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

5 similar comments
@akalenyu
Copy link
Collaborator Author

akalenyu commented Jun 7, 2023

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

@akalenyu
Copy link
Collaborator Author

akalenyu commented Jun 7, 2023

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

@akalenyu
Copy link
Collaborator Author

akalenyu commented Jun 8, 2023

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

@akalenyu
Copy link
Collaborator Author

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

@akalenyu
Copy link
Collaborator Author

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

@akalenyu akalenyu changed the title [WIP] Debug test failure - missing pod following recreate of CDI Fix test failure - missing pod following recreate of CDI Jun 12, 2023
@akalenyu akalenyu marked this pull request as ready for review June 12, 2023 09:10
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 12, 2023
@akalenyu akalenyu marked this pull request as draft June 12, 2023 09:45
@kubevirt-bot kubevirt-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 12, 2023
@akalenyu akalenyu changed the title Fix test failure - missing pod following recreate of CDI [WIP] Fix test failure - missing pod following recreate of CDI Jun 12, 2023
@akalenyu
Copy link
Collaborator Author

@awels I think what happens is that the priority class tests leave old replicas of the cdi-uploadproxy/deployment/apiserver
since they make those change.. and thus the initial CDI pod list is corrupted

@awels
Copy link
Member

awels commented Jun 12, 2023

Okay, I guess we need to make sure those are cleaned up before moving to the next test then

@akalenyu
Copy link
Collaborator Author

Okay, I guess we need to make sure those are cleaned up before moving to the next test then

Yeah just have to find a nice way that relies on the deployment status

@akalenyu
Fix Destructive test lane failure - missing pod following recreate of…
17e06f8 
… CDI

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
@akalenyu akalenyu marked this pull request as ready for review June 18, 2023 15:02
@kubevirt-bot kubevirt-bot requested a review from awels June 18, 2023 15:02
@akalenyu akalenyu changed the title [WIP] Fix test failure - missing pod following recreate of CDI Fix test failure - missing pod following recreate of CDI Jun 18, 2023
@kubevirt-bot kubevirt-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 18, 2023
@akalenyu akalenyu mentioned this pull request Jun 22, 2023
Merged
@akalenyu
Copy link
Collaborator Author

Okay, I guess we need to make sure those are cleaned up before moving to the next test then

Yeah just have to find a nice way that relies on the deployment status

Should be ok now

@akalenyu
Copy link
Collaborator Author

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

@akalenyu
Copy link
Collaborator Author

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

ShellyKa13
ShellyKa13 reviewed Jun 26, 2023
View reviewed changes
Copy link
Contributor

@ShellyKa13 ShellyKa13 left a comment

Choose a reason for hiding this comment

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

/lgtm

@kubevirt-bot kubevirt-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 26, 2023
@mhenriks
Copy link
Member

/approve

@kubevirt-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mhenriks

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 Jun 26, 2023
@kubevirt-bot kubevirt-bot merged commit a9d213b into kubevirt:main Jun 27, 2023
1 check passed
akalenyu added a commit to akalenyu/containerized-data-importer that referenced this pull request Jul 3, 2023
@akalenyu
Fix Destructive test lane failure - missing pod following recreate of…
df23243 
… CDI (kubevirt#2744)

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
kubevirt-bot added a commit that referenced this pull request Jul 6, 2023
@akalenyu @akrejcir
@mhenriks @maya-r @kubevirt-bot @avlitman
[release-v1.57] Backport main commits to 1.57 release branch v2 (#2785)
50efcee 
* dataimportcron: Pass dynamic credential support label (#2760)

* dataimportcron: code change: Use better matchers in tests

Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>

* dataimportcron: Pass dynamic credential support label

The label is passed from DataImportCron to DataVolume
and DataSource.

Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>

---------

Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>

* Add DataImportCron snapshot sources docs (#2747)

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* add akalenyu as approver, some others as reviewers (#2766)

Signed-off-by: Michael Henriksen <mhenriks@redhat.com>

* Run `make rpm-deps` (#2741)

* Run make rpm-deps

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Avoid overlayfs error message by using vfs driver

Signed-off-by: Maya Rashish <mrashish@redhat.com>

---------

Signed-off-by: Maya Rashish <mrashish@redhat.com>

* Fix Destructive test lane failure - missing pod following recreate of CDI (#2744)

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* [WIP] Handle nil ptr in dataimportcron controller (#2769)

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* Revert some gomega error checking that produce confusing output (#2772)

One of these tests flakes, but the error is hard to debug because
gomega will yell about
`Unexpected non-nil/non-zero argument at index 0`
Instead of showing the error.

Apparently this is intended:
https://github.com/onsi/gomega/pull/480/files#diff-e696deff1a5be83ad03053b772926cb325cede3b33222fa76c2bb1fcf2efd809R186-R190

Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>

* Run bazelisk run //robots/cmd/uploader:uploader -- -workspace /home/prow/go/src/github.com/kubevirt/project-infra/../containerized-data-importer/WORKSPACE -dry-run=false (#2770)

Signed-off-by: kubevirt-bot <kubevirtbot@redhat.com>

* [CI] Add metrics name linter (#2774)

Signed-off-by: Aviv Litman <alitman@redhat.com>

---------

Signed-off-by: Andrej Krejcir <akrejcir@redhat.com>
Signed-off-by: Alex Kalenyuk <akalenyu@redhat.com>
Signed-off-by: Michael Henriksen <mhenriks@redhat.com>
Signed-off-by: Maya Rashish <mrashish@redhat.com>
Signed-off-by: kubevirt-bot <kubevirtbot@redhat.com>
Signed-off-by: Aviv Litman <alitman@redhat.com>
Co-authored-by: Andrej Krejcir <akrejcir@gmail.com>
Co-authored-by: Michael Henriksen <mhenriks@redhat.com>
Co-authored-by: Maya Rashish <mrashish@redhat.com>
Co-authored-by: kubevirt-bot <kubevirtbot@redhat.com>
Co-authored-by: Aviv Litman <64130977+avlitman@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShellyKa13 ShellyKa13 ShellyKa13 left review comments

@maya-r maya-r Awaiting requested review from maya-r

@mhenriks mhenriks Awaiting requested review from mhenriks

@aglitke aglitke Awaiting requested review from aglitke

@awels awels Awaiting requested review from awels

Assignees

@ShellyKa13 ShellyKa13

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/S
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@akalenyu @kubevirt-bot @awels @mhenriks @ShellyKa13