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

bors: macOS integration tests always time out #12050

Closed
abhinav opened this issue Feb 2, 2023 · 3 comments
Closed

bors: macOS integration tests always time out #12050

abhinav opened this issue Feb 2, 2023 · 3 comments
Assignees
Labels
area/build CI/CD for pulumi/pulumi kind/engineering Work that is not visible to an external user os/darwin p0 A bug severe enough to interrupt existing work resolution/fixed This issue was fixed
Milestone

Comments

@abhinav
Copy link
Contributor

abhinav commented Feb 2, 2023

@abhinav abhinav added area/build CI/CD for pulumi/pulumi kind/engineering Work that is not visible to an external user p1 A bug severe enough to be the next item assigned to an engineer os/darwin labels Feb 2, 2023
@abhinav
Copy link
Contributor Author

abhinav commented Feb 3, 2023

Experimenting in #12053

@Frassle Frassle added p0 A bug severe enough to interrupt existing work and removed p1 A bug severe enough to be the next item assigned to an engineer labels Feb 3, 2023
abhinav added a commit that referenced this issue Feb 3, 2023
In #11155, we pinned CI to macos-11
because we want to continue to support macOS 11.

This, however hsa created a new issue:
grpcio does not publish wheels for macos-11 anymore
and building it from source takes long enough to kill our CI
(see #12054 #12050).

To fix the issue while continuing to support older macOS,
run CI on macos-latest, but leave build jobs and friends on macos-11.

Resolves #12054, #12050
abhinav added a commit that referenced this issue Feb 3, 2023
In #11155, we pinned CI to macos-11
because we want to continue to support macOS 11.

This, however hsa created a new issue:
grpcio does not publish wheels for macos-11 anymore
and building it from source takes long enough to kill our CI
(see #12054 #12050).

To fix the issue while continuing to support older macOS,
run CI on macos-latest, but leave build jobs and friends on macos-11.

Resolves #12054, #12050
abhinav added a commit that referenced this issue Feb 3, 2023
In #11155, we pinned CI to macos-11
because we want to continue to support macOS 11.

This, however hsa created a new issue:
grpcio does not publish wheels for macos-11 anymore
and building it from source takes long enough to kill our CI
(see #12054 #12050).

To fix the issue while continuing to support older macOS,
run CI on macos-latest, but leave build jobs and friends on macos-11.

Resolves #12054, #12050
bors bot added a commit that referenced this issue Feb 3, 2023
12066: ci: Use macos-latest r=abhinav a=abhinav

In #11155, we pinned CI to macos-11
because we want to continue to support macOS 11.

This, however hsa created a new issue:
grpcio does not publish wheels for macos-11 anymore
and building it from source takes long enough to kill our CI
(see #12054 #12050).

To fix the issue while continuing to support older macOS,
run CI on macos-latest, but leave build jobs and friends on macos-11.

Resolves #12054, #12050


Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
abhinav added a commit that referenced this issue Feb 3, 2023
Issue #12050 would have been a lot easier to debug
if we had been capturing the output of the commands.

This changes runComponentSetup to execute bash with `-x`
so that every command in the shell script is logged before it's run.

This feeds that output into an `io.Writer` that logs to the testing.TB.
This way, if the test fails or times out,
we'll have a bunch of meaningful log output
associated with the tests that ran them.

Capturing the command output in a reliable way
required introduction of something slightly more complicated
than `strings.Split(\n)` because there's a good chance
that we'll have partial lines written to the writer.

The approach taken by the Writer here is:
it will log the input one line at a time usually,
but when it sees a partial line, it will buffer it in-memory
to be combined with the next write or flush.
Approach inspired by https://github.com/uber-go/zap/blob/v1.24.0/zapio/writer.go

Refs #12050
@abhinav
Copy link
Contributor Author

abhinav commented Feb 3, 2023

Resolved by #12066

@abhinav abhinav closed this as completed Feb 3, 2023
@pulumi-bot pulumi-bot reopened this Feb 3, 2023
@pulumi-bot
Copy link
Contributor

Cannot close issue without required labels: resolution/

@abhinav abhinav added the resolution/fixed This issue was fixed label Feb 3, 2023
@abhinav abhinav closed this as completed Feb 3, 2023
abhinav added a commit that referenced this issue Feb 3, 2023
Issue #12050 would have been a lot easier to debug
if we had been capturing the output of the commands.

This changes runComponentSetup to execute bash with `-x`
so that every command in the shell script is logged before it's run.

This feeds that output into an `io.Writer` that logs to the testing.TB.
This way, if the test fails or times out,
we'll have a bunch of meaningful log output
associated with the tests that ran them.

Capturing the command output in a reliable way
required introduction of something slightly more complicated
than `strings.Split(\n)` because there's a good chance
that we'll have partial lines written to the writer.

The approach taken by the Writer here is:
it will log the input one line at a time usually,
but when it sees a partial line, it will buffer it in-memory
to be combined with the next write or flush.
Approach inspired by https://github.com/uber-go/zap/blob/v1.24.0/zapio/writer.go

Refs #12050
bors bot added a commit that referenced this issue Feb 4, 2023
12028: Require linting before running unit, integ, and smoke tests. r=abhinav a=RobbieMcKinstry

<!--- 
Thanks so much for your contribution! If this is your first time contributing, please ensure that you have read the [CONTRIBUTING](https://github.com/pulumi/pulumi/blob/master/CONTRIBUTING.md) documentation.
-->

# Description

**Update:** With #12031 linting runs in about a minute.

Linting should be an extremely low-watermark requirement for evaluating build health. Blocking on it allows use to reduce the number of concurrent runners who are canceled early.

**Trade-offs:**
* This should delay CI time by the amount of time it takes to lint: _e.g._ CI will be ~5 minutes slower on the happy path.
* When a job is queued that fails a lint check, fewer runners will be soaked up just to fail lint checks. This will decrease the overall queue time across all builds.
* Ultimately, we're trading slower happy-path builds for smarter build scheduling. 
* We can mitigate the linting bottleneck by speeding up the linting process (#12023).

<!--- Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. -->

This PR supports but isn't sufficient for #12019

## Checklist

**This PR is intended to impact CI only, and thus does not justify a CHANGELOG entry or a test.**

<!--- Please provide details if the checkbox below is to be left unchecked. -->
- [ ] I have added tests that prove my fix is effective or that my feature works
<!--- 
User-facing changes require a CHANGELOG entry.
-->
- [ ] I have run `make changelog` and committed the `changelog/pending/<file>` documenting my change
<!--
If the change(s) in this PR is a modification of an existing call to the Pulumi Service,
then the service should honor older versions of the CLI where this change would not exist.
You must then bump the API version in /pkg/backend/httpstate/client/api.go, as well as add
it to the service.
-->
- [ ] Yes, there are changes in this PR that warrants bumping the Pulumi Service API version
  <!-- `@Pulumi` employees: If yes, you must submit corresponding changes in the service repo. -->


12043: sdk/go: Don't store DependsOn in a lossy form r=abhinav a=abhinav



The `DependsOn` and `DependsOnInputs` resource options
store their captured information on the `resourceOptions` struct
in a lossy format: they store function references.

This makes it impossible to go back to the original lists of resources
or resource array inputs for use cases like #11698.

As a step towards making this possible,
replace the stored closures with interfaces.

The implementations in the first commit
are a drop-in replacement for the prior behavior
with no logic changes whatsoever.

The second commit makes a minor optimization:
it adds URNs to the same set instead
of constantly allocating new sets and combining them afterwards.

Refs #11698


12048: ci/test-docs-generation: Attempt to use version-sets r=abhinav a=abhinav

Addresses the TODO and uses version-set generated by get-job-matrix
to decide which version of our various platforms should be used.


12069: test/integration: Capture component setup output r=abhinav a=abhinav

Issue #12050 would have been a lot easier to debug
if we had been capturing the output of the commands.

This changes runComponentSetup to execute bash with `-x`
so that every command in the shell script is logged before it's run.

This feeds that output into an `io.Writer` that logs to the testing.TB.
This way, if the test fails or times out,
we'll have a bunch of meaningful log output
associated with the tests that ran them.

Capturing the command output in a reliable way
required introduction of something slightly more complicated
than `strings.Split(\n)` because there's a good chance
that we'll have partial lines written to the writer.

The approach taken by the Writer here is:
it will log the input one line at a time usually,
but when it sees a partial line, it will buffer it in-memory
to be combined with the next write or flush.
Approach inspired by https://github.com/uber-go/zap/blob/v1.24.0/zapio/writer.go

Refs #12050


Co-authored-by: Robbie McKinstry <robbie@pulumi.com>
Co-authored-by: Abhinav Gupta <abhinav@pulumi.com>
abhinav added a commit that referenced this issue Feb 4, 2023
Enforces the timeout for runComponentSetup on external commands
by using CommandContext.
If the command runs past the duration allowed by the context,
it will be terminated without ceremony.
Per the os/exec docs:

> The provided context is used to interrupt the process
> (by calling cmd.Cancel or os.Process.Kill)
> if the context becomes done before the command completes on its own.

The default `cmd.Cancel` is Process.Kill, which:

> Kill causes the Process to exit immediately.
> Kill does not wait until the Process has actually exited.
> This only kills the Process itself, not any other processes it may have started.

Refs #12050
phillipedwards pushed a commit that referenced this issue Feb 8, 2023
In #11155, we pinned CI to macos-11
because we want to continue to support macOS 11.

This, however hsa created a new issue:
grpcio does not publish wheels for macos-11 anymore
and building it from source takes long enough to kill our CI
(see #12054 #12050).

To fix the issue while continuing to support older macOS,
run CI on macos-latest, but leave build jobs and friends on macos-11.

Resolves #12054, #12050
phillipedwards pushed a commit that referenced this issue Feb 8, 2023
Issue #12050 would have been a lot easier to debug
if we had been capturing the output of the commands.

This changes runComponentSetup to execute bash with `-x`
so that every command in the shell script is logged before it's run.

This feeds that output into an `io.Writer` that logs to the testing.TB.
This way, if the test fails or times out,
we'll have a bunch of meaningful log output
associated with the tests that ran them.

Capturing the command output in a reliable way
required introduction of something slightly more complicated
than `strings.Split(\n)` because there's a good chance
that we'll have partial lines written to the writer.

The approach taken by the Writer here is:
it will log the input one line at a time usually,
but when it sees a partial line, it will buffer it in-memory
to be combined with the next write or flush.
Approach inspired by https://github.com/uber-go/zap/blob/v1.24.0/zapio/writer.go

Refs #12050
@mikhailshilkov mikhailshilkov added this to the 0.84 milestone Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build CI/CD for pulumi/pulumi kind/engineering Work that is not visible to an external user os/darwin p0 A bug severe enough to interrupt existing work resolution/fixed This issue was fixed
Projects
None yet
Development

No branches or pull requests

4 participants