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.

Sign up for GitHub

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

Jump to bottom

test(profiler): use go 1.18.4 in integration test #6348

Merged
merged 5 commits into from Jul 18, 2022
Merged

test(profiler): use go 1.18.4 in integration test #6348

merged 5 commits into from Jul 18, 2022

Conversation

amchiclet
Copy link
Contributor

@amchiclet amchiclet commented Jul 15, 2022 •
edited

Also switching to use gimme (which is well tested and more robust) for the Go installation.

And also printing the go version before running the test, which is useful for debugging when something goes wrong.

@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: cloudprofiler Issues related to the Cloud Profiler API. labels Jul 15, 2022
@amchiclet amchiclet marked this pull request as ready for review July 15, 2022 22:32
@amchiclet amchiclet requested review from a team as code owners July 15, 2022 22:32
@amchiclet amchiclet requested a review from wyk9787 July 16, 2022 00:12
@product-auto-label product-auto-label bot added size: s Pull request size is small. and removed size: xs Pull request size is extra small. labels Jul 16, 2022
@amchiclet amchiclet changed the title test(profiler): log go version in integration test test(profiler): use go 1.18.4 in integration test Jul 16, 2022
codyoss
codyoss reviewed Jul 18, 2022
View reviewed changes
profiler/kokoro/integration_test.sh
retry curl -sL -o "$GIMME" https://raw.githubusercontent.com/travis-ci/gimme/master/gimme
chmod +x "$GIMME"

export GIMME_GO_VERSION=1.18.4
Copy link
Member

Choose a reason for hiding this comment

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

Our CI environment already tests on the max supported version and earliest supported version. For instance latest is already using 1.18. Maybe instead of re-downloading the toolchain you just detect that you are running from the latest job. I get there is a kokoro envvar you can read for this information.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback! I think you had similar feedback like last time.

These tests are extra, on top of the general CI tests that are performed in google-cloud-go.

The Go version being used in kokoro is v1.12. We hope to have a more controlled and custom VM for our tests, but unfortunately for now we will still have to use what kokoro provides.

Copy link
Member

Choose a reason for hiding this comment

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

@amchiclet I would consider looking into trampoline. You can bootstrap your env with a container. That is how the common CI works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would love to do things the recommended way. Thanks for the pointer and will look into trampoline when we get a chance.

@amchiclet amchiclet merged commit 761768b into googleapis:main Jul 18, 2022
@amchiclet
Copy link
Contributor Author

I went ahead an merged this change. If needed, further improvements can be in follow up PRs.

aalexand
aalexand reviewed Jul 18, 2022
View reviewed changes
profiler/kokoro/integration_test.sh
"$GIMME"
# If gimme was successful, an .env script that sets up proper environment
# variables will be created.
if [[ -f "${GIMME_ENV_PREFIX}/go${GIMME_GO_VERSION}.env" ]]
Copy link
Contributor

Choose a reason for hiding this comment

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

The script uses "set -eo pipefail", so is this conditional really necessary? If the gimme command fails we wouldn't get here. And even if we do, why not just do source "${GIMME_ENV_PREFIX}/go${GIMME_GO_VERSION}.env" unconditionally and let the command fail if the file is not there, which should fail the script?

Or is this to enable retries? It is not obvious from the comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this can be cleaned up as you suggested. Thanks for pointing this out.

noahdietz added a commit that referenced this pull request Jul 18, 2022
@yoshi-code-bot @cojenco
@shollyman @codyoss @yoshi-automation @hongalex @release-please @tritone @noahdietz @gcf-merge-on-green @amchiclet
chore: sync main to storage-refactor (#6356)
07be81e 
* feat(storage): add Custom Placement Config Dual Region Support  (#6294)

* feat(storage): support Custom Dual Regions with CustomPlacementConfig

* fix typo

* add comments

* address pr comments

* new sublink

* feat(bigquery/storage/managedwriter/adapt): support packed field option (#6312)

* feat(bigquery/storage/managedwriter/adapt): support packed field option

This PR adds the "packed" field option for repeated numeric scalar types
when converting from table schema to proto descriptor.  For large
repetitions, this can yield wire size encoding benefits.

This option is only relevant for proto2 descriptors; proto3 packs by
default.

* chore: update go version to 1.17 (#6342)

This does not affect the version of Go we support. More details
here: googleapis/go-genproto#859

* chore(all): auto-regenerate gapics (#6337)

This is an auto-generated regeneration of the gapic clients by
cloud.google.com/go/internal/gapicgen. Once the corresponding genproto PR is
submitted, genbot will update this PR with a newer dependency to the newer
version of genproto and assign reviewers to this PR.

If you have been assigned to review this PR, please:

- Ensure that the version of genproto in go.mod has been updated.
- Ensure that CI is passing. If it's failing, it requires your manual attention.
- Approve and submit this PR if you believe it's ready to ship.

Corresponding genproto PR: googleapis/go-genproto#857

Changes:

feat(bigquery/migration): Add Presto dialect to bigquerymigration v2 client library
  PiperOrigin-RevId: 460797158
  Source-Link: googleapis/googleapis@46f2598

* fix(bigquery/storage/managedwriter): improve network reconnection (#6338)

* fix(bigquery/storage/managedwriter): improve network reconnection

Issuing a sufficiently large single append request is enough to trigger
the server backend to close an existing grpc stream.  This PR addresses
the problem by allowing a failed request to signal that subsequent
requests should request a new grpc stream connection.

This PR also adds an integration test that induces the failure by
issuing a large request, and ensures subsequent requests succeed.

Towards: #6321

* fix(pubsub): make receipt modack call async (#6335)

* fix(pubsub): make receipt modack call async

* dont propagate modack errors

* update comment on why errors are not checked

* chore(all): auto-regenerate gapics (#6347)

This is an auto-generated regeneration of the gapic clients by
cloud.google.com/go/internal/gapicgen. Once the corresponding genproto PR is
submitted, genbot will update this PR with a newer dependency to the newer
version of genproto and assign reviewers to this PR.

If you have been assigned to review this PR, please:

- Ensure that the version of genproto in go.mod has been updated.
- Ensure that CI is passing. If it's failing, it requires your manual attention.
- Approve and submit this PR if you believe it's ready to ship.

Corresponding genproto PR: googleapis/go-genproto#861

Changes:

feat(dataplex): Add IAM support for Explore content APIs feat: Add support for custom container for Task feat: Add support for cross project for Task feat: Add support for custom encryption key to be used for encrypt data on the PDs associated with the VMs in your Dataproc cluster for Task feat: Add support for Latest job in Task resource feat: User mode filter in Explore list sessions API feat: Support logging sampled file paths per partition to Cloud logging for Discovery event
  PiperOrigin-RevId: 461116673
  Source-Link: googleapis/googleapis@9af1b9b

* chore(main): release bigquery 1.36.0 (#6343)

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

* chore(storage): remove adapters dependency (#6350)

The go-type-adapters package is used only minimally in storage,
and nowhere else in google-cloud-go. Seems easiest to just drop
this dependency.

* doc(pubsub): clarify behavior of ack deadlines (#6301)

* doc(pubsub): clarify behavior of ack deadlines

* add 99th percentile documentation

* chore(ci): add sync branch workflow for storage-refactor (#6334)

* chore(ci): add sync branch workflow for storage-refactor

* add change notes

* only on weekdays

* change job names

* add cloud-storage-dpe to reviewers

Co-authored-by: gcf-merge-on-green[bot] <60162190+gcf-merge-on-green[bot]@users.noreply.github.com>

* test(profiler): use go 1.18.4 in integration test (#6348)

* chore: fix renovate (#6353)

It has not processed in 20 days. I think we need to be careful with
the PRs and have renovate try to do all the rebasing needed or else
it seems to get confused and not able to recover.

Fixes: #6352

* chore(main): release pubsub 1.24.0 (#6300)

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Alex Hong <9397363+hongalex@users.noreply.github.com>

* chore(ci): storage-refactor sync use Yoshi Code Bot (#6355)

Co-authored-by: gcf-merge-on-green[bot] <60162190+gcf-merge-on-green[bot]@users.noreply.github.com>

Co-authored-by: cojenco <cathyo@google.com>
Co-authored-by: shollyman <shollyman@google.com>
Co-authored-by: Cody Oss <6331106+codyoss@users.noreply.github.com>
Co-authored-by: Yoshi Automation Bot <yoshi-automation@google.com>
Co-authored-by: Alex Hong <9397363+hongalex@users.noreply.github.com>
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Chris Cotter <cjcotter@google.com>
Co-authored-by: Noah Dietz <noahdietz@users.noreply.github.com>
Co-authored-by: gcf-merge-on-green[bot] <60162190+gcf-merge-on-green[bot]@users.noreply.github.com>
Co-authored-by: Amarin (Um) Phaosawasdi <amchiclet@users.noreply.github.com>
noahdietz pushed a commit that referenced this pull request Jul 18, 2022
@amchiclet @noahdietz
test(profiler): use go 1.18.4 in integration test (#6348)
6bb4fb8
noahdietz added a commit that referenced this pull request Jul 19, 2022
@noahdietz
Revert "test(profiler): use go 1.18.4 in integration test (#6348)"
9292955 
This reverts commit 6bb4fb8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aalexand aalexand aalexand left review comments

@codyoss codyoss codyoss left review comments

@wyk9787 wyk9787 wyk9787 approved these changes

@nolanmar511 nolanmar511 Awaiting requested review from nolanmar511

Assignees
No one assigned
Labels
api: cloudprofiler Issues related to the Cloud Profiler API. size: s Pull request size is small.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@amchiclet @aalexand @codyoss @wyk9787