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

Unittests for await logic (Delete) #2800

Closed
Tracked by #2774 ...
EronWright opened this issue Jan 31, 2024 · 0 comments · Fixed by #3012
Closed
Tracked by #2774 ...

Unittests for await logic (Delete) #2800

EronWright opened this issue Jan 31, 2024 · 0 comments · Fixed by #3012
Assignees
Labels
kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed

Comments

@EronWright
Copy link
Contributor

EronWright commented Jan 31, 2024

Develop unit test cases for the Delete function of the await package.

@mjeffryes mjeffryes added the kind/engineering Work that is not visible to an external user label Feb 5, 2024
@mjeffryes mjeffryes assigned blampe and unassigned rquitales Apr 2, 2024
@EronWright EronWright assigned EronWright and unassigned blampe May 17, 2024
blampe added a commit that referenced this issue May 17, 2024
This adds await logic for DaemonSets with RollingUpdate or OnDelete
update strategies.

The implementation is largely based on the existing StatefulSet logic
with two high-level simplifications:

1. We use
[kstatus](https://pkg.go.dev/sigs.k8s.io/cli-utils/pkg/kstatus/status)
to decide when a DaemonSet is ready.
2. We use a `PodAggregator` to handle reporting pod statuses.

Importantly, unlike StatefulSet this means we do not currently inspect
pods to decide readiness -- we only use them for informational purposes.
I _think_ this is sufficient but I could easily be missing something. I
haven't been able to simulate situations where this logic doesn't fully
capture readiness and we would need to inspect pod statuses.

A failing e2e test was added in YAML under the awkwardly name 
`tests/sdk/java` path.

Unit tests were added around the public `Creation`, `Update`, etc.
methods in order to more fully exercise timeouts and retries. To that
end I introduced a mock clock package which might be controversial. IMO
Go doesn't have a great OSS mock clock but something like this can be
very helpful for testing.

I'm still somewhat confused by the role of `await.Read` since it doesn't
actually await anything, but it's implemented similar to StatefulSet as
a one-shot read + readiness check.

Fixes #609
Refs #2800
Refs #2799
Refs #2798
EronWright added a commit that referenced this issue May 20, 2024
<!--Thanks for your contribution. See [CONTRIBUTING](CONTRIBUTING.md)
    for Pulumi's contribution guidelines.

    Help us merge your changes more quickly by adding more details such
    as labels, milestones, and reviewers.-->

### Proposed changes

<!--Give us a brief description of what you've done and what it solves.
-->

### Related issues (optional)

<!--Refer to related PRs or issues: #1234, or 'Fixes #1234' or 'Closes
#1234'.
Or link to full URLs to issues or pull requests in other GitHub
repositories. -->

Closes #2800
@pulumi-bot pulumi-bot added the resolution/fixed This issue was fixed label May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/engineering Work that is not visible to an external user resolution/fixed This issue was fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants