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

[TT-1984] Use job distributor in keystone smoke test + FMS change #16169

Merged
merged 9 commits into from
Feb 3, 2025

Conversation

Tofel
Copy link
Contributor

@Tofel Tofel commented Jan 31, 2025

CORE:

  • allows to register gateway and standardcapabilities jobs with the FMS
  • adds integration tests for finding these two new jobs

Keystone smoke test:

  • use Job Distributor to create jobs

Unclear:

  • gateway job selection criteria
  • should we support other number types than int64, when iterating over untyped Gateway job spec?

@Tofel Tofel requested review from a team as code owners January 31, 2025 17:24
@Tofel Tofel requested a review from patrickhuie19 January 31, 2025 17:25
Copy link
Contributor

github-actions bot commented Jan 31, 2025

AER Report: CI Core ran successfully ✅

aer_workflow , commit

AER Report: Operator UI CI ran successfully ✅

aer_workflow , commit

@@ -823,6 +823,79 @@ func (s *GatewaySpec) SetID(value string) error {
return nil
}

// NodeServerConfigPath returns NodeServerConfig.Path or empty string, if not found or it's not a string
func (s *GatewaySpec) NodeServerConfigPath() string {
if nsc, ok := s.GatewayConfig["NodeServerConfig"]; ok {
Copy link
Contributor

Choose a reason for hiding this comment

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

@Tofel I'd recommend just unmarshaling the config into a struct (I would expect this to already exist somewhere) and then just grabbing what you need from it; same with the function below.

Copy link
Contributor Author

@Tofel Tofel Feb 3, 2025

Choose a reason for hiding this comment

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

Okay, I'll use

type GatewayConfig struct {
, didn't know it existed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, can't use due to circular imports :/ and I don't want to put that and other types in yet another package to resolve it, I think it should be done in another PR

@cedric-cordenier
Copy link
Contributor

@Tofel I'd also recommend running this by someone who is more familiar with Gateways, eg. @bolekk @justinkaseman

@Tofel Tofel requested review from bolekk and justinkaseman February 3, 2025 13:34
justinkaseman
justinkaseman previously approved these changes Feb 3, 2025
@Tofel Tofel dismissed stale reviews from justinkaseman and cedric-cordenier via 57856f9 February 3, 2025 17:48
@Tofel Tofel enabled auto-merge February 3, 2025 18:37
@justinkaseman justinkaseman self-requested a review February 3, 2025 18:40
@Tofel Tofel added this pull request to the merge queue Feb 3, 2025
Merged via the queue into develop with commit 9e7579f Feb 3, 2025
185 checks passed
@Tofel Tofel deleted the tt-1984-jd-keystone-smoke branch February 3, 2025 19:03
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.

3 participants