Extend client APIs for support of existing stacks in more cases #2338
Conversation
jsoriano
force-pushed
the
enrollment-token-apis
branch
from
ddc3a23 to
4e1ccea
Compare
| outputID := "" | ||
| if settings.LogstashEnabled { | ||
| outputID = serverless.FleetLogstashOutput | ||
| } | ||
|
|
||
| logger.Infof("Creating agent policy") | ||
| err = project.CreateAgentPolicy(ctx, sp.kibanaClient, options.StackVersion, outputID, settings.SelfMonitor) | ||
| _, err = createAgentPolicy(ctx, sp.kibanaClient, options.StackVersion, outputID, settings.SelfMonitor) |
There was a problem hiding this comment.
Not for this PR, probably this method could be reused in system/tester.go to create the agent policies too.
| project, err = sp.currentProjectWithClientsAndFleetEndpoint(ctx, config) | ||
| if err != nil { | ||
| return fmt.Errorf("failed to retrieve latest project created: %w", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
Nice! One less request (or two requests depending if there is default a fleetserver or not).
| Name: id, | ||
| ID: id, |
There was a problem hiding this comment.
Could this fail if tests are run in parallel? Or maybe this will not fail but it will use an unexpected output. Or maybe it is good to reuse the same output when running in parallel.
Or well, the caller is the one to take into account assigning the right/expected IDs.
There was a problem hiding this comment.
Yes, this can be an issue if we use the same stack for different environments, for example if a serverless cluster with two environments, one with logstash and the other one without it. But it should not be an issue though while running tests in parallel with a single environment.
For reusing a stack for different executions we have more conflicts. I would leave fixing them for future improvements. One issue we have is that we are relying on default Fleet configurations (output, fleet server, fleet server policy...), and I think we could avoid it.
I think that ideally each environment should use its own resources, and they should be explicitly set in agent policies, and don't rely on defaults.
| if err != nil { | ||
| return nil, fmt.Errorf("failed to parse Kibana version (%s): %w", c.versionInfo.Number, err) | ||
| // Version info may not contain any version if this is a managed Kibana. | ||
| if c.versionInfo.Number != "" { |
There was a problem hiding this comment.
Could this happen in Serverless projects ? Should we set a default version in that case or just not set it at all ?
There was a problem hiding this comment.
Yes, this happens in serverless.
Not sure of what would be better.
In my other branch I was setting 9.0.0 (here). But it can be tricky to chose a proper version, and remember to update it if it is not something like 99.99.99.
So here I am opting by no setting it at all, what indicates that it is a deployment with an unknown version. The only known case for this is serverless, so it is in principle safe to assume that is the latest version. The problem with this approach is that we have to remember to do null checks.
An alternative I considered was to create an interface with the features that are dependant of the version, and set the proper implementation depending on the info here.
There was a problem hiding this comment.
So here I am opting by no setting it at all, what indicates that it is a deployment with an unknown version. The only known case for this is serverless, so it is in principle safe to assume that is the latest version.
It looks reasonable to assume that, ok 👍
Co-authored-by: Mario Rodriguez Molins <marrodmo@gmail.com>
Co-authored-by: Mario Rodriguez Molins <marrodmo@gmail.com>
jsoriano
force-pushed
the
enrollment-token-apis
branch
from
6cadd80 to
cc789a8
Compare
💛 Build succeeded, but was flaky
Failed CI StepsHistory
cc @jsoriano |
This change moves some methods from the serverless provider to the clients and adds some additional methods that can be useful to extend the support of existing stacks.
Fleet Server:
Elasticsearch:
Kibana:
This is part of #2298.