-
Notifications
You must be signed in to change notification settings - Fork 115
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
Experimental CI improvements #2244
Conversation
Does the PR have any schema changes?Looking good! No breaking changes found. |
e6ca46a
to
e0fa1ea
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
e0fa1ea
to
00535d9
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
00535d9
to
f86b216
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
f86b216
to
abe0a37
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
1 similar comment
Does the PR have any schema changes?Looking good! No breaking changes found. |
47847a7
to
50ba7a4
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
1 similar comment
Does the PR have any schema changes?Looking good! No breaking changes found. |
b56e0bd
to
c0f876a
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
I looked at the failing test, and it's related to tests using a Service with type: LoadBalancer. |
The build_sdk and test jobs both use a matrix to run a similar series of steps for each language (Java, go, etc.). But it's not necessary for every step to run for every language; in particular, it's not necessary to set up any language other than the one under consideration, and Go at the set version to run the tests.
There's a couple of problems with running a cluster in GCP to be the target of tests: - it means all the tests use the same cluster, and they all run concurrently -- though this doesn't seem to be a big problem in practice - you can't rerun any individual test suite, because of the way GitHub Actions work -- once a dependency has succeeded, it won't get run again. To be able to re-run a job, the cluster really needs to be spun up within the job.
Some of the tests (I'm looking at you, TestDotnet_CustomResource) use APIs which have been removed from recent versions of Kubernetes (respectively, apiextensions/v1beta1, removed in Khbernetes v1.22 August 2021). The SDK supports deprecated and removed APIs, so it's valid to use a removed version; but, it means we need to run an old cluster for the test to pass. Obviously there's a tension between testing old APIs and testing with up-to-date clusters -- I've erred on the side of not changing everything at once. Signed-off-by: Michael Bridgen <[email protected]>
Signed-off-by: Michael Bridgen <[email protected]>
The versions of each language compiler/runtime are given in matrix entries in individual steps. This produces the right results, but it makes the workflow structure confusing and obscures what's actually running (since each matrix value is put in the job name). Since there's only one version used for each language, and it's the same for every step, they may as well be workflow-level env entries. Signed-off-by: Michael Bridgen <[email protected]>
c0f876a
to
035ec02
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
efa3208
to
933ea7d
Compare
- Use file targets where there are files being built, so that `make` knows where it can avoid rebuilding things - Mark phony targets as phony. Generally, the phony targets are to give CI a way to call into the makefile when building in stages - Replace double-colon rules with normal rules. There's no reason to use double-colon rules here. Signed-off-by: Michael Bridgen <[email protected]>
Taking a leaf from pulumi-azure-native: instead of trying to calculate the provider version in the Makefile, expect it to be passed in the environment or as a Makefile variable, with a (valid) dummy value by default. This means the CI can calculate the version for use elsewhere (e.g., for publishing), and be sure it's the same value used here. Also: use pulumictl to convert the provider version to each of SDK versions. To make sure a pulumictl with convert-version is used, download it as part of `make ensure`. Signed-off-by: Michael Bridgen <[email protected]>
933ea7d
to
795ed2f
Compare
Does the PR have any schema changes?Looking good! No breaking changes found. |
Previously, it was assumed `pulumictl` would be somewhere in the PATH, and the CI scripts explicitly installed it. To avoid coincidences, the Makefile now uses `pulumictl` specifically from `./bin/pulumictl`, and `make ensure` will download it there -- so the CI workflows need to use this too. Signed-off-by: Michael Bridgen <[email protected]>
Does the PR have any schema changes?Looking good! No breaking changes found. |
Signed-off-by: Michael Bridgen <[email protected]>
Does the PR have any schema changes?Looking good! No breaking changes found. |
The test suites now run against kind, which is easier to overwhelm. Signed-off-by: Michael Bridgen <[email protected]>
Does the PR have any schema changes?Looking good! No breaking changes found. |
Another experiment in improving the CI to e.g., solve #2243. Changes would ultimately need to be made in pulumi/ci-mgmt and propagated here; but that's a very circuitous way to try things out.