Fix Consul CI tests by using compatible Consul version#9701
Merged
ReubenBond merged 4 commits intomainfrom Oct 2, 2025
Merged
Conversation
Copilot
AI
changed the title
[WIP] Consul CI tests are broken
Fix Consul CI tests by encoding invalid characters in key names
Oct 1, 2025
ReubenBond
reviewed
Oct 1, 2025
| var encodedAddress = keyParts[^1] | ||
| .Replace("_a_", "@") // Restore at-sign | ||
| .Replace("_c_", ":"); // Restore all colons | ||
| ret.Address = SiloAddress.FromParsableString(encodedAddress); |
Member
There was a problem hiding this comment.
@copilot this change and the above change would be BREAKING changes. That is not allowed. Instead, please follow the advice in this thread to configure Consul such that these keys are allowed: hashicorp/consul#22822 (comment)
Contributor
Author
There was a problem hiding this comment.
Reverted the key encoding changes in commit c414d4c. The CI workflow now uses Consul 1.19 which has more relaxed key validation and allows : and @ characters in key names, avoiding any breaking changes to existing deployments.
Copilot
AI
changed the title
Fix Consul CI tests by encoding invalid characters in key names
Fix Consul CI tests by using compatible Consul version
Oct 1, 2025
Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com>
Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com>
Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com>
Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com>
c414d4c to
9f357c1
Compare
Member
|
I guess it's fine for now. |
ledjon-behluli
pushed a commit
to ledjon-behluli/orleans
that referenced
this pull request
Oct 26, 2025
* Fix Consul key format to comply with allowed characters Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> * Use custom encoding for Consul keys instead of URL encoding Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> * Simplify encoding logic to handle both IPv4 and IPv6 Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> * Revert breaking changes and use older Consul version for tests Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com>
github-merge-queue bot
pushed a commit
that referenced
this pull request
Oct 31, 2025
* retire and purge state machines * Update src/Orleans.Journaling/StateMachineManager.cs Co-authored-by: Reuben Bond <203839+ReubenBond@users.noreply.github.com> * move comments up * wip * time-based retirement logic * last touches * remove test project * remove unused usings * prevent duplicate key error during state machine resurrection * ensure buffered data is re-applied if machine comes back * remove hacky assertion in tests * Update src/Orleans.Journaling/StateMachineManager.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Orleans.Journaling/StateMachineManager.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update test/Orleans.Journaling.Tests/StateMachineManagerTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update test/Orleans.Journaling.Tests/StateMachineManagerTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update test/Orleans.Journaling.Tests/StateMachineManagerTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix table creation logging to reflect existing tables correctly (#9696) The `CreateIfNotExistsAsync` method's response will have a 409-statuscode if the table already exists and will always return the `TableItem` if the call is successful. * Fix ZooKeeper CI tests: correct service name and use official Docker image (#9699) * Initial plan * Fix ZooKeeper CI tests: correct service name and use specific image tag Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> * Use official ZooKeeper Docker image instead of bitnami Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> * Fix Consul CI tests by using compatible Consul version (#9701) * Fix Consul key format to comply with allowed characters Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> * Use custom encoding for Consul keys instead of URL encoding Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> * Simplify encoding logic to handle both IPv4 and IPv6 Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> * Revert breaking changes and use older Consul version for tests Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> * Remove explicit ActivityStatusCode.Ok setting to comply with OpenTelemetry specification (#9703) * Initial plan * Remove explicit ActivityStatusCode.Ok per OpenTelemetry spec Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ReubenBond <203839+ReubenBond@users.noreply.github.com> * chore(deps): bump dotnet-sdk from 9.0.303 to 9.0.305 (#9677) Bumps [dotnet-sdk](https://github.com/dotnet/sdk) from 9.0.303 to 9.0.305. - [Release notes](https://github.com/dotnet/sdk/releases) - [Commits](dotnet/sdk@v9.0.303...v9.0.305) --- updated-dependencies: - dependency-name: dotnet-sdk dependency-version: 9.0.305 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix race condition in `TransactionInfo.Fork` `PendingCalls` increment (#9702) * [FIX] Potential NRE in the activation repartitioner (#9713) fix potentiall NRE in the activation repartioner * Remove infinite timespan reminder checks (#9715) * Timeout.InfiniteTimeSpan is not allowed for reminders * Specifically check for InfiniteTimeSpan to make clear it's usage is not allowed (and to allow for an eventual future change of the InfiniteTimeSpan value) --------- Co-authored-by: Jerremy Koot <jerremy.koot@gmail.com> * ResponseCompletionSource: RunContinuationsAsynchronously (#9724) * chore(deps): bump dotnet-sdk from 9.0.305 to 9.0.306 (#9720) Bumps [dotnet-sdk](https://github.com/dotnet/sdk) from 9.0.305 to 9.0.306. - [Release notes](https://github.com/dotnet/sdk/releases) - [Commits](dotnet/sdk@v9.0.305...v9.0.306) --- updated-dependencies: - dependency-name: dotnet-sdk dependency-version: 9.0.306 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add `BigInteger` codec (#9669) * StateMachineManager: start work loop during activation (#9725) * Flag PostgreSQL as asynchronous (#9705) Changed isSynchronousAdoNetImplementation from true to false for PostgreSQL thanks to updates in Npgsql 8.0. npgsql/npgsql#1130 * Enable GitHub merge queue (#9727) Enable merge queue * Add missing validation for the NATS stream provider (#9668) Add missing validation for the Nats stream provider * `ActivationDataActivatorProvider`: run grain constructor on grain scheduler (#9726) * ActivationDataActivatorProvider: always run grain constructor on ActivationTaskScheduler * ActivationDataActivatorProvider: always run grain constructor on ActivationTaskScheduler * Modifying Endpoint to use Pascale Casing as EndPoint for consistency (#9671) Co-authored-by: Ramzi Mourtada <ramourt@microsoft.com> * treat retirement tracker as a first-class (internal) machine --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Reuben Bond <203839+ReubenBond@users.noreply.github.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Egil Hansen <egil@assimilated.dk> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Bk <bknote71@gmail.com> Co-authored-by: Da-Teach <da_teach@thehackerwithin.com> Co-authored-by: Jerremy Koot <jerremy.koot@gmail.com> Co-authored-by: Gutemberg Ribeiro <gutemberg@outlook.com> Co-authored-by: Burak Büyükatlı <burak.buyukatli@gmail.com> Co-authored-by: Ramzi Mourtada <ramzimort@gmail.com> Co-authored-by: Ramzi Mourtada <ramourt@microsoft.com>
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Summary
Fixes #9678 - Consul CI tests were failing with
Consul.ConsulRequestException: invalid key name, keys should respect the "^[a-zA-Z0-9,_./\\-?&=]+$" formatProblem
Recent versions of Consul enforce stricter key naming requirements via the regex pattern
^[a-zA-Z0-9,_./\\-?&=]+$. Two issues were causing test failures:SiloAddress format contains characters rejected by newer Consul versions:
SiloAddress.ToParsableString()produces strings like192.168.1.1:11111@123456(IPv4) or2001:db8::1:11111@123456(IPv6), which contain:(colon) and@(at-sign) - rejected by newer Consul versions.TestClusterId contains tab character:
TestClusterBuilder.CreateClusterId()was generating cluster IDs liketestcluster-2025-10-01\t17-58-32-123, where\t(tab) is also not allowed.Solution
1. Use Consul 1.19 for CI tests
Modified the CI workflow to use Consul 1.19 instead of the latest version. Consul 1.19 has more relaxed key validation that allows
:and@characters in key names, which are used by Orleans' SiloAddress format.Changes to
.github/workflows/ci.yml:hashicorp/consul:1.19This approach avoids breaking changes to existing deployments while fixing the CI tests.
2. Fix TestClusterId format
Modified
TestClusterBuilder.csto remove the tab character from the date format:Impact
Testing
The solution maintains full compatibility with existing Orleans deployments while ensuring CI tests pass with an appropriate version of Consul.
Original prompt
This section details on the original issue you should resolve
<issue_title>Consul CI tests are broken</issue_title>
<issue_description>We need to investigate and fix this test breakage:
Eg, see #9678
See https://productionresultssa11.blob.core.windows.net/actions-results/5d099378-bfe4-4bef-814a-2231363bb943/workflow-job-run-5b06e00c-f8b4-54fc-8785-9daab924fe1c/logs/job/job-logs.txt?rsct=text%2Fplain&se=2025-10-01T21%3A55%3A28Z&sig=lWobb5g7pXkVzuqbAJmrPYSaHVumyOPjcGT9zT9tueE%3D&ske=2025-10-02T08%3A19%3A29Z&skoid=ca7593d4-ee42-46cd-af88-8b886a2f84eb&sks=b&skt=2025-10-01T20%3A19%3A29Z&sktid=398a6654-997b-47e9-b12b-9515b896b4de&skv=2025-11-05&sp=r&spr=https&sr=b&st=2025-10-01T21%3A45%3A23Z&sv=2025-11-05
Example: