roachtest: update TeamCity links to the new UI. #81103
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This change is |
renatolabs
added
the
first-pr
tbg
approved these changes
May 6, 2022
There was a problem hiding this comment.
If we're missing one it's not the end of the world.
Good question about slack.go. It is still being "plumbed around":
$ git grep slack-token -- build/
build/teamcity-nightly-roachtest.sh: --slack-token="${SLACK_TOKEN}" \
build/teamcity/cockroach/nightlies/pebble_nightly_write_throughput_impl.sh: --slack-token "${SLACK_TOKEN-}" \
build/teamcity/cockroach/nightlies/pebble_nightly_ycsb_impl.sh: --slack-token "${SLACK_TOKEN-}" \
build/teamcity/cockroach/nightlies/pebble_nightly_ycsb_race_impl.sh: --slack-token "${SLACK_TOKEN-}" \
build/teamcity/cockroach/nightlies/roachtest_nightly_impl.sh: --slack-token="${SLACK_TOKEN}" \
But I agree - what's going on in #production indicates that this isn't actually posting from any job (and if it did, it would be useless).
I think we could ignore this for now, and possibly even remove the slack support (though we have to be careful with that, since if we remove the flag without also updating all scripts that call into roachtest, the invocations will fail). I have not heard anyone clamor for being notified, in Slack, of failures of individual tests; folks are struggling enough to keep up with the issues.
This seems lower priority though. If you're not sure what to pick up next, and find that cleanup satisfying, I would be happy to review it.
Reviewed 26 of 26 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @nicktrav)
nicktrav
approved these changes
May 6, 2022
There was a problem hiding this comment.
TIL about the "new" TC interface.
Deferring to Tobi for the "official" LGTM, but
Reviewed 26 of 26 files at r1, all commit messages.
Reviewable status:
srosenberg
approved these changes
May 6, 2022
There was a problem hiding this comment.
Reviewed 26 of 26 files at r1, all commit messages.
Reviewable status:
Makes sense, I'll come back to this when the right mood strikes, maybe on a Friday. |
It's been added to all builds under "Cockroach" on TeamCity (commit). |
In 2019, TeamCity released a new "experimental UI" that changed not
only the look and feel of the app, but also the URLs where pages are
located. Notably, information about a build used to be under the
/viewLog.html page in the old UI, but was moved to
/buildConfiguration/{buildTypeID}/{buildID} in the new UI.
If the user enabled the new UI locally, TeamCity will automatically
redirect requests from the old URLs to the new ones; however, the URL
fragments are not preserved. This is quite bad for roachtest, since
the links generated in failed test reports relies on the fragments in
order to expand the relevant folders in a potentially long list of
artifacts.
This commit updates TeamCity links generated by roachtest when
reporting failed tests to always use the new UI. It has the drawback
of forcing the new UI on everyone, but the benefit of always expanding
the correct folder.
A new environment variable, `TC_BUILDTYPE_ID`, was added to TeamCity
builds to support this use case; it contains the ID of the
corresponding build type (also known as "build configuration").
Resolves: cockroachdb#80852.
Release note: None.
renatolabs
force-pushed
the
80852-roachtest-teamcity-link-fix
branch
from
847d92a
to
2237cce
Compare
|
bors r=tbg,nicktrav,srosenberg |
|
Build failed (retrying...): |
|
Build succeeded: |
renatolabs
added a commit
to renatolabs/cockroach
that referenced
this pull request
May 20, 2022
In cockroachdb#81103, the process of generating TeamCity links in test failure reports started relying on the `TC_BUILDTYPE_ID` environment variable. While that variable was added to TeamCity builds, it was not being passed down to Docker where the tests actually run. As a result, links generated by the GitHub poster were broken (see, for example, cockroachdb#81572). This commit makes `TC_BUILDTYPE_ID` accessible by Docker for every build that was already passing `TC_BUILD_BRANCH`. This should be sufficient to cover all existing cases and more, in case having access to this variable becomes useful in the future. Release note: None
craig bot
pushed a commit
that referenced
this pull request
May 20, 2022
81389: vendor: bump Pebble to e567fec84c6e r=jbowens a=jbowens This commit includes non-trivial changes to account for the change in the `vfs.WithDiskHealthChecks` function signature change. Additionally, this PR uncovered #81413 and necessitated some changes to work around it. ``` e567fec8 db: ensure Open closes opened directories on error b8c9a560 internal/metamorphic: overwrite unused bounds buffers 7c5f0cbb db: copy user-provided bounds and optimize unchanged bounds f8897076 *: Add IterOption to optionally read L6 filter blocks. d79f9617 vfs: extend disk-health checking to filesystem metadata operations 5ae21746 db: remove newRangeKeyIter closure 6d975489 db: add BenchmarkIteratorScan 782d102e sstable: fix invariant check for sstable size estimation 738a7f0b db: fix NewIter regression resulting in extra memtable levels 37558663 *: Use keyspan.LevelIter for rangedels in compactions e6c60c71 db: use sublevel level iters for all compactions out of L0 d8f63bef db: extend documentation on ingested file overlap; improve test cases b9e970a8 internal/keyspan: correct and document MergingIter key stability 498177bb internal/keyspan: collapse fragmentBoundIterator into MergingIter ``` Release note (bug fix): Fix a gap in disk-stall detection. Previously, disk stalls during filesystem metadata operations could go undetected, inducing deadlocks. Now stalls during these types of operations will correctly fatal the process. 81535: colmem: improve the behavior of ResetMaybeReallocate r=yuzefovich a=yuzefovich Previously, `Allocator.ResetMaybeReallocate` would always grow the capacity of the batch until `coldata.BatchSize()` (unless the memory limit has been exceeded). This behavior allows us to have batches with dynamic size when we don't know how many rows we need to process (for example, in the `cFetcher` we start out with 1 row, then grow it exponentially - 2, 4, 8, etc). However, in some cases we know exactly how many rows we want to include into the batch, so that behavior can result in us re-allocating a batch needlessly when the old batch already have enough capacity. This commit improves the situation by adding a knob to indicate that if the desired capacity is satisfied by the old batch, then it should not be re-allocated. All callers have been audited accordingly. Release note: None 81552: roachtest: fix pgjdbc and typeorm r=otan a=rafiss fixes #81515 fixes #81431 See individual commits. 81558: sql,stmtdiagnostics: remove some version gates r=yuzefovich a=yuzefovich This commit addresses several TODOs with my name on them about removing the version gates, mostly around the conditional statement diagnostics introduced in 22.1 cycle. Release note: None 81579: roachtest: allow TC_BUILDTYPE_ID to be accessible by Docker r=rickystewart a=renatolabs In #81103, the process of generating TeamCity links in test failure reports started relying on the `TC_BUILDTYPE_ID` environment variable. While that variable was added to TeamCity builds, it was not being passed down to Docker where the tests actually run. As a result, links generated by the GitHub poster were broken (see, for example, #81572). This commit makes `TC_BUILDTYPE_ID` accessible by Docker for every build that was already passing `TC_BUILD_BRANCH`. This should be sufficient to cover all existing cases and more, in case having access to this variable becomes useful in the future. Release note: None Co-authored-by: Jackson Owens <jackson@cockroachlabs.com> Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com> Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com> Co-authored-by: Renato Costa <renato@cockroachlabs.com>
andrewbaptist
pushed a commit
to andrewbaptist/cockroach
that referenced
this pull request
May 25, 2022
In cockroachdb#81103, the process of generating TeamCity links in test failure reports started relying on the `TC_BUILDTYPE_ID` environment variable. While that variable was added to TeamCity builds, it was not being passed down to Docker where the tests actually run. As a result, links generated by the GitHub poster were broken (see, for example, cockroachdb#81572). This commit makes `TC_BUILDTYPE_ID` accessible by Docker for every build that was already passing `TC_BUILD_BRANCH`. This should be sufficient to cover all existing cases and more, in case having access to this variable becomes useful in the future. Release note: None
This was referenced Jan 17, 2023
srosenberg
pushed a commit
to srosenberg/cockroach
that referenced
this pull request
Jan 23, 2023
In cockroachdb#81103, the process of generating TeamCity links in test failure reports started relying on the `TC_BUILDTYPE_ID` environment variable. While that variable was added to TeamCity builds, it was not being passed down to Docker where the tests actually run. As a result, links generated by the GitHub poster were broken (see, for example, cockroachdb#81572). This commit makes `TC_BUILDTYPE_ID` accessible by Docker for every build that was already passing `TC_BUILD_BRANCH`. This should be sufficient to cover all existing cases and more, in case having access to this variable becomes useful in the future. Release note: None Epic: none
srosenberg
pushed a commit
to srosenberg/cockroach
that referenced
this pull request
Jan 23, 2023
In cockroachdb#81103, the process of generating TeamCity links in test failure reports started relying on the `TC_BUILDTYPE_ID` environment variable. While that variable was added to TeamCity builds, it was not being passed down to Docker where the tests actually run. As a result, links generated by the GitHub poster were broken (see, for example, cockroachdb#81572). This commit makes `TC_BUILDTYPE_ID` accessible by Docker for every build that was already passing `TC_BUILD_BRANCH`. This should be sufficient to cover all existing cases and more, in case having access to this variable becomes useful in the future. Release note: None Epic: none
srosenberg
pushed a commit
to srosenberg/cockroach
that referenced
this pull request
Jan 23, 2023
In cockroachdb#81103, the process of generating TeamCity links in test failure reports started relying on the `TC_BUILDTYPE_ID` environment variable. While that variable was added to TeamCity builds, it was not being passed down to Docker where the tests actually run. As a result, links generated by the GitHub poster were broken (see, for example, cockroachdb#81572). This commit makes `TC_BUILDTYPE_ID` accessible by Docker for every build that was already passing `TC_BUILD_BRANCH`. This should be sufficient to cover all existing cases and more, in case having access to this variable becomes useful in the future. Release note: None Epic: none
smg260
pushed a commit
to smg260/cockroach
that referenced
this pull request
Jan 31, 2023
In cockroachdb#81103, the process of generating TeamCity links in test failure reports started relying on the `TC_BUILDTYPE_ID` environment variable. While that variable was added to TeamCity builds, it was not being passed down to Docker where the tests actually run. As a result, links generated by the GitHub poster were broken (see, for example, cockroachdb#81572). This commit makes `TC_BUILDTYPE_ID` accessible by Docker for every build that was already passing `TC_BUILD_BRANCH`. This should be sufficient to cover all existing cases and more, in case having access to this variable becomes useful in the future. Release note: None
smg260
pushed a commit
to smg260/cockroach
that referenced
this pull request
Feb 2, 2023
In cockroachdb#81103, the process of generating TeamCity links in test failure reports started relying on the `TC_BUILDTYPE_ID` environment variable. While that variable was added to TeamCity builds, it was not being passed down to Docker where the tests actually run. As a result, links generated by the GitHub poster were broken (see, for example, cockroachdb#81572). This commit makes `TC_BUILDTYPE_ID` accessible by Docker for every build that was already passing `TC_BUILD_BRANCH`. This should be sufficient to cover all existing cases and more, in case having access to this variable becomes useful in the future. Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In 2019, TeamCity released a new "experimental UI" that changed not
only the look and feel of the app, but also the URLs where pages are
located. Notably, information about a build used to be under the
/viewLog.html page in the old UI, but was moved to
/buildConfiguration/{buildTypeID}/{buildID} in the new UI.
If the user enabled the new UI locally, TeamCity will automatically
redirect requests from the old URLs to the new ones; however, the URL
fragments are not preserved. This is quite bad for roachtest, since
the links generated in failed test reports relies on the fragments in
order to expand the relevant folders in a potentially long list of
artifacts.
This commit updates TeamCity links generated by roachtest when
reporting failed tests to always use the new UI. It has the drawback
of forcing the new UI on everyone, but the benefit of always expanding
the correct folder.
A new environment variable,
TC_BUILDTYPE_ID, was added to TeamCitybuilds to support this use case; it contains the ID of the
corresponding build type (also known as "build configuration").
Resolves: #80852.
Release note: None.
@tbg: I also saw that slack.go generates a link to the old UI. Is that code being used at the moment? It seems to try to send notifications to the
#productionSlack channel, but the notifications I see there don't seem to match what the code in that file is doing.