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

Deflake should enable and disable the registry-cache extension TM test #123

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

ialidzhikov
Copy link
Member

@ialidzhikov ialidzhikov commented Jan 5, 2024

How to categorize this PR?

/area testing
/kind bug

What this PR does / why we need it:
I see many failures in the should enable and disable the registry-cache extension TM test.
The reason for the failures is that in the step Disable the registry-cache extension

By("Disable the registry-cache extension")
Expect(f.UpdateShoot(ctx, func(shoot *gardencorev1beta1.Shoot) error {
common.RemoveRegistryCacheExtension(shoot)
return nil
})).To(Succeed())

ctx is used. However ctx is already a context of another step Wait until the registry configuration is applied:

By("Wait until the registry configuration is applied")
ctx, cancel = context.WithTimeout(parentCtx, 5*time.Minute)
defer cancel()
common.WaitUntilRegistryConfigurationsAreApplied(ctx, f.Logger, f.ShootClient)

After step Wait until the registry configuration is applied the step Verify registry-cache works is executed (can take up to 12mins) and currently the step Disable the registry-cache extension is executed with context that has less than 5min timeout (it can be the case that the timeout is already exceeded meanwhile during the next step execution).

This PR fixes this by creating a new context from the parentContext with timeout of 10mins for the Disable the registry-cache extension step.

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:
Logs proving the flake:

STEP: Enable the registry-cache extension @ 01/04/24 09:35:36.34
STEP: Wait until the registry configuration is applied @ 01/04/24 09:39:06.837
STEP: Verify registry-cache works @ 01/04/24 09:39:28.049
STEP: Create nginx Pod @ 01/04/24 09:39:28.049
STEP: Wait until nginx Pod is running @ 01/04/24 09:39:28.061
STEP: Verify the registry cache pulled the nginx image @ 01/04/24 09:39:43.095
STEP: Delete nginx Pod @ 01/04/24 09:39:43.716
STEP: Disable the registry-cache extension @ 01/04/24 09:39:48.739

[FAILED] in [It] - /src/test/testmachinery/shoot/enable_disable_test.go:70 @ 01/04/24 09:44:06.855

Note that the failure is exactly atWait until the registry configuration is applied + 5min.

Release note:

A flake in the `should enable and disable the registry-cache extension` testmachinery test is now fixed.

@gardener-prow gardener-prow bot added area/testing Testing related kind/bug Bug labels Jan 5, 2024
@gardener-prow gardener-prow bot added the cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. label Jan 5, 2024
@gardener-prow gardener-prow bot requested a review from timebertt January 5, 2024 13:12
@gardener-prow gardener-prow bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jan 5, 2024
@dimitar-kostadinov dimitar-kostadinov self-assigned this Jan 8, 2024
@plkokanov
Copy link

/lgtm

@gardener-prow gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 9, 2024
Copy link
Contributor

gardener-prow bot commented Jan 9, 2024

LGTM label has been added.

Git tree hash: 98b561579b91a20518ec060834be66e2e68f2645

Copy link
Contributor

@dimitar-kostadinov dimitar-kostadinov left a comment

Choose a reason for hiding this comment

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

/lgtm

@ialidzhikov
Copy link
Member Author

/approve

Copy link
Contributor

gardener-prow bot commented Jan 9, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ialidzhikov

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

@gardener-prow gardener-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 9, 2024
@gardener-prow gardener-prow bot merged commit ad4fc47 into gardener:main Jan 9, 2024
6 checks passed
@ialidzhikov ialidzhikov deleted the fix/tm-test branch January 10, 2024 06:30
@JordanJordanov JordanJordanov added area/ipcei IPCEI (Important Project of Common European Interest) ipcei/registry-cache-extension Registry Cache Extension labels May 21, 2024
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. area/ipcei IPCEI (Important Project of Common European Interest) area/testing Testing related cla: yes Indicates the PR's author has signed the cla-assistant.io CLA. ipcei/registry-cache-extension Registry Cache Extension kind/bug Bug lgtm Indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants