Skip to content

orchestratord test: Introduce upgrade scenario #33461

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
Merged
merged 5 commits into from
Oct 23, 2025
Merged

orchestratord test: Introduce upgrade scenario #33461

merged 5 commits into from
Oct 23, 2025

Conversation

@def-
Copy link
Contributor

@def- def- commented Aug 28, 2025

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered. (trigger-ci for additional test/nightly runs)
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • If this PR includes major user-facing behavior changes, I have pinged the relevant PM to schedule a changelog post.

@def- def- force-pushed the pr-orchestratord-upgrade branch 6 times, most recently from b218e73 to cc9e7a8 Compare September 2, 2025 18:06
@def- def- mentioned this pull request Sep 2, 2025
Merged
5 tasks
@def- def- force-pushed the pr-orchestratord-upgrade branch 6 times, most recently from ba1a409 to 0f263dc Compare September 4, 2025 13:41
@def- def- marked this pull request as ready for review September 4, 2025 13:41
@def- def- requested a review from a team as a code owner September 4, 2025 13:41
@def-
Copy link
Contributor Author

def- commented Sep 4, 2025

I had to disable a few checks with TODO, as well as the combined upgrade scenarios, since they reliably fail and I haven't yet investigated if it's expected. There are somehow more failures than I expected initially.

alex-hunt-materialize
alex-hunt-materialize reviewed Sep 4, 2025
View reviewed changes
test/orchestratord/mzcompose.py
definition["materialize2"] = copy.deepcopy(definition["materialize"])
definition["materialize2"]["metadata"][
"name"
] = "12345678-1234-1234-1234-123456789013"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not need to point at separate backend_secret_name and environment_id in the Materialize object? If we don't, won't they be pointing at the same database?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This metadata name is the backend_secret_name. I added a comment for the pg db.

test/orchestratord/mzcompose.py Outdated
definition["operator"]["telemetry"]["enabled"] = self.value

def validate(self, mods: dict[type[Modification], Any]) -> None:
return # TODO: Doesn't work with upgrade: Expected no --segment-api-key= in environmentd args, but found it
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these all orchestratord bugs? Should we be version gating these args?

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 don't know. I was hoping your or @doy-materialize would know!

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 have now opened https://github.com/MaterializeInc/database-issues/issues/9697

test/orchestratord/mzcompose.py
definition["operator"]["operator"]["tag"] = get_tag(args.tag)
# makes environmentd -> clusterd connections fail
# definition["operator"]["networkPolicies"]["enabled"] = True
# definition["operator"]["networkPolicies"]["internal"]["enabled"] = True
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a bug report for this? It should work. I typically do my testing using the cloud kind cluster and:

networkPolicies:
  enabled: true
  internal:
    enabled: true
  ingress:
    enabled: true
  egress:
    enabled: true

This usually works for me.

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 have opened https://github.com/MaterializeInc/database-issues/issues/9696

@def- def- force-pushed the pr-orchestratord-upgrade branch from 0f263dc to 13080d9 Compare September 17, 2025 14:52
@def- def- force-pushed the pr-orchestratord-upgrade branch 3 times, most recently from e716e34 to 19d83a6 Compare September 23, 2025 11:40
@def- def- marked this pull request as draft September 23, 2025 11:55
@def- def- force-pushed the pr-orchestratord-upgrade branch 4 times, most recently from 8deecae to 223edd0 Compare September 23, 2025 13:52
@def- def- force-pushed the pr-orchestratord-upgrade branch from 79403c7 to 9f59ca9 Compare October 3, 2025 23:02
@def- def- marked this pull request as ready for review October 3, 2025 23:03
@def-
Copy link
Contributor Author

def- commented Oct 3, 2025

We should get this green and merge this week, I'm giving it another try (at the airport): https://buildkite.com/materialize/nightly/builds/13658

@def- def- mentioned this pull request Oct 3, 2025
Closed
5 tasks
@def- def- force-pushed the pr-orchestratord-upgrade branch 2 times, most recently from 86c7870 to 184b911 Compare October 3, 2025 23:39
@def- def- force-pushed the pr-orchestratord-upgrade branch from 184b911 to aa837e3 Compare October 21, 2025 12:45
@def- def- marked this pull request as draft October 21, 2025 12:46
@def- def- force-pushed the pr-orchestratord-upgrade branch 8 times, most recently from c4af185 to 973e012 Compare October 22, 2025 12:13
@def- def- force-pushed the pr-orchestratord-upgrade branch from 973e012 to 7926862 Compare October 22, 2025 13:07
@alex-hunt-materialize alex-hunt-materialize dismissed their stale review October 22, 2025 14:01

It's been a month of significant changes since I reviewed.

@def- def- marked this pull request as ready for review October 22, 2025 15:48
@def-
Copy link
Contributor Author

def- commented Oct 22, 2025

It's a miracle: https://buildkite.com/materialize/nightly/builds/13835
Screenshot 2025-10-22 at 17 48 45

@def-
Copy link
Contributor Author

def- commented Oct 23, 2025

Ready for review again.

alex-hunt-materialize
alex-hunt-materialize approved these changes Oct 23, 2025
View reviewed changes
Copy link
Contributor

@alex-hunt-materialize alex-hunt-materialize left a comment

Choose a reason for hiding this comment

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

There are several Modifications that are disabled for versions they should work on. Lets get this merged, and we can iterate on them after that.

@def- def- merged commit 68a566c into MaterializeInc:main Oct 23, 2025
138 checks passed
@def- def- deleted the pr-orchestratord-upgrade branch October 23, 2025 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alex-hunt-materialize alex-hunt-materialize alex-hunt-materialize approved these changes

@doy-materialize doy-materialize Awaiting requested review from doy-materialize

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@def- @alex-hunt-materialize