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 endless PVC await #3130

Merged
merged 5 commits into from
Jul 25, 2024
Merged

Fix endless PVC await #3130

merged 5 commits into from
Jul 25, 2024

Conversation

blampe
Copy link
Contributor

@blampe blampe commented Jul 24, 2024

This fixes an issue where WaitForFirstConsumer PVCs could cause deadlocks due to the downstream consumer waiting for the PVC to be bound.

We now attempt to fetch the PVC's storage class and the corresponding volumeBindMode. If we can find the bind mode, then we consider the PVC ready once it's Pending. The existing behavior is preserved if we can't find the bind mode, or if it's Immediate.

An e2e test is included. I'm omitting unit tests as this might end up getting refactored in some later PRs.

Fixes #895.

@blampe blampe requested review from rquitales and EronWright July 24, 2024 23:40
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

Attention: Patch coverage is 4.76190% with 40 lines in your changes missing coverage. Please review.

Project coverage is 36.24%. Comparing base (641e0e6) to head (ca7b3d2).

Files Patch % Lines
provider/pkg/await/awaiters.go 0.00% 39 Missing ⚠️
provider/pkg/await/deployment.go 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3130      +/-   ##
==========================================
- Coverage   36.39%   36.24%   -0.16%     
==========================================
  Files          70       70              
  Lines        9226     9251      +25     
==========================================
- Hits         3358     3353       -5     
- Misses       5531     5559      +28     
- Partials      337      339       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

CHANGELOG.md Outdated Show resolved Hide resolved
provider/pkg/await/awaiters.go Outdated Show resolved Hide resolved
provider/pkg/await/awaiters.go Show resolved Hide resolved
@blampe blampe requested a review from rquitales July 25, 2024 20:00
@blampe blampe merged commit 645df55 into master Jul 25, 2024
19 checks passed
@blampe blampe deleted the blampe/895-pvc-first-consumer branch July 25, 2024 23:18
blampe added a commit that referenced this pull request Aug 7, 2024
### Added

- `clusterIdentifier` configuration can now be used to manually control
the replacement behavior of a provider resource.
(#3068)

- Pod errors now include the pod's last termination state, as well as
the pod's termination message if available.
(#3091)

The pod's termination message can be helpful in `CrashLoopBackOff`
situations but will only be reported if it was correctly configured.

By default, the pod's termination message is read from
`/dev/termination-log`. This location can be configured with
`terminationMessagePath`.

Use `terminationMessagePolicy: FallbackToLogsOnError` to use the pod's
logs as its termination message.

- Documentation is now generated for all languages supported by overlay
types. (#3107)

### Fixed

- Updated logic to accurately detect if a resource is a Patch variant.
(#3102)
- Added Java as a supported language for `CustomResource` overlays.
(#3120)
- Status messages reported during updates are now more accurately scoped
to the
affected resource.
(#3128)
- `PersistentVolumeClaims` with a bind mode of `WaitForFirstConsumer`
will no
longer hang indefinitely.
(#3130)
- [java] Fixed an issue where child resources could not be registered by
Chart v4. (#3119)
lumiere-bot bot referenced this pull request in coolguy1771/home-ops Aug 8, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@pulumi/kubernetes](https://pulumi.com)
([source](https://togithub.com/pulumi/pulumi-kubernetes)) | dependencies
| minor | [`4.15.0` ->
`4.16.0`](https://renovatebot.com/diffs/npm/@pulumi%2fkubernetes/4.15.0/4.16.0)
|

---

### Release Notes

<details>
<summary>pulumi/pulumi-kubernetes (@&#8203;pulumi/kubernetes)</summary>

###
[`v4.16.0`](https://togithub.com/pulumi/pulumi-kubernetes/blob/HEAD/CHANGELOG.md#4160-August-7-2024)

[Compare
Source](https://togithub.com/pulumi/pulumi-kubernetes/compare/v4.15.0...v4.16.0)

##### Added

- `clusterIdentifier` configuration can now be used to manually control
the
replacement behavior of a provider
resourc[https://github.com/pulumi/pulumi-kubernetes/pull/3068](https://togithub.com/pulumi/pulumi-kubernetes/pull/3068)ull/3068)

- Pod errors now include the pod's last termination state, as well as
the pod's
termination message if
availabl[https://github.com/pulumi/pulumi-kubernetes/pull/3091](https://togithub.com/pulumi/pulumi-kubernetes/pull/3091)ull/3091)

The pod's termination message can be helpful in `CrashLoopBackOff`
situations but
    will only be reported if it was correctly configured.

    By default, the pod's termination message is read from
    `/dev/termination-log`. This location can be configured with
    `terminationMessagePath`.

Use `terminationMessagePolicy: FallbackToLogsOnError` to use the pod's
logs
    as its termination message.

- Documentation is now generated for all languages supported by overlay
types.

[https://github.com/pulumi/pulumi-kubernetes/pull/3107](https://togithub.com/pulumi/pulumi-kubernetes/pull/3107)3107)

##### Fixed

- Updated logic to accurately detect if a resource is a Patch variant.
([https://github.com/pulumi/pulumi-kubernetes/pull/3102](https://togithub.com/pulumi/pulumi-kubernetes/pull/3102))
- Added Java as a supported language for `CustomResource` overlays.
([https://github.com/pulumi/pulumi-kubernetes/pull/3120](https://togithub.com/pulumi/pulumi-kubernetes/pull/3120))
- Status messages reported during updates are now more accurately scoped
to the
affected
resourc[https://github.com/pulumi/pulumi-kubernetes/pull/3128](https://togithub.com/pulumi/pulumi-kubernetes/pull/3128)3128)
- `PersistentVolumeClaims` with a bind mode of `WaitForFirstConsumer`
will no
longer hang
indefinitel[https://github.com/pulumi/pulumi-kubernetes/pull/3130](https://togithub.com/pulumi/pulumi-kubernetes/pull/3130)3130)
- \[java] Fixed an issue where child resources could not be registered
by Chart v4.
[https://github.com/pulumi/pulumi-kubernetes/pull/3119](https://togithub.com/pulumi/pulumi-kubernetes/pull/3119)9)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Renovate
Bot](https://togithub.com/renovatebot/renovate).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC4yMS4zIiwidXBkYXRlZEluVmVyIjoiMzguMjEuMyIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsidHlwZS9taW5vciJdfQ==-->

Co-authored-by: lumiere-bot[bot] <98047013+lumiere-bot[bot]@users.noreply.github.com>
@mjeffryes mjeffryes added this to the 0.108 milestone Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot use PVC (eternal wait)
3 participants