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.

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

[v9.17.0] Maestro v9 to coexist with other Game Room providers #623

Open
wants to merge 6 commits into
base: v9.16.0
Choose a base branch
from

Conversation

hspedro
Copy link
Collaborator

@hspedro hspedro commented Jun 5, 2024

  • refactor(controller): do not fail scheduler if namespace exists

When creating a scheduler, we fail if the namespace already exists. This is problematic if there are other GR providers running at the same namespace, like maestro v10 for example. Thus, don't fail scheduler creation if namespace exists.

  • refactor(controller): do not delete namespace on scheduler delete

There can be other GR providers running into the same namespace, thus we don't want to cascade delete the k8s namespace on scheduler deletion

  • refactor(watcher): filter pod events by heritage label

There can be other resources/pods running in the namespace that are not managed by Maestro. In that case, we want to filter the watcher to only receive pod events from the ones created by v9

@hspedro hspedro changed the base branch from main to v9.16.0 June 5, 2024 14:51
@hspedro hspedro force-pushed the feat/filter-events-by-heritage branch from 8f9f4a6 to 597ef9c Compare June 5, 2024 14:59
@coveralls
Copy link

Coverage Status

coverage: 78.143% (+0.06%) from 78.079%
when pulling 597ef9c on feat/filter-events-by-heritage
into 4338d9d on v9.16.0.

@hspedro hspedro force-pushed the feat/filter-events-by-heritage branch from 597ef9c to f2c4677 Compare June 5, 2024 15:11
@coveralls
Copy link

Coverage Status

coverage: 78.077% (-0.002%) from 78.079%
when pulling 367abb6 on feat/filter-events-by-heritage
into 4338d9d on v9.16.0.

@hspedro hspedro changed the title [v9.16.0] Filter room events by heritage label on pods [v9.17.0] Maestro v9 to coexist with other Game Room providers Jun 5, 2024
@reinaldooli
Copy link
Collaborator

Wdyt about adding a configuration option for the namespace deletion? My concern here is that when not having other GR providers, it's good to delete te namespace.

Copy link
Collaborator

@joaobologna joaobologna left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -840,7 +837,8 @@ func UpdateSchedulerConfig(
}

// MustUpdatePods returns true if it's necessary to delete old pod and create a new one
// so this have the new configuration.
//
// so this have the new configuration.
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this intended?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants