feat(agent-binary): simplestream finder in stub

[ycliuhw]

This PR is implement a new function on the stubdomain service that allows the caller to get the models BootStrapEnviron. We have avoid leaking out the provider for a model out of the domain services now. Because we are in a time crunch we are going to allow it to be fetched via a domain service so that it can be supplied to the ToolsFinder facade. This is needed so we can feed this into environ tools for fetching agent binaries.

Checklist

• Code style: imports ordered, good names, simple structure, etc
• Comments saying why design decisions were made
• Go unit tests, with comments saying what you're testing
• Integration tests, with comments saying what you're testing
• doc.go added or updated in changed packages

QA steps

TODO

Documentation changes

No

Links

Jira card:

• JUJU-7781
• JUJU-7783

[tlm]

Hey @ycliuhw,

Thanks for the work on this. Can I point out that this task doesn't require us to wire up the dependency but just that we need to supply it out of the stub service. We have a task coming up to perform the wire up.

What I was trying to explain is that if we provide a wrapped env tools finder we can leave the finding code in the apiserver and we don't have to do most of this work at the moment. We can leave it for when we are truly ready to do the full extent of the re-design.

In the stub service this is all the task is currently expecting:

type EnvironToolsFinder interface {
	FindTools(context.Context, int, int, []string, coretools.Filter) (coretools.List, error)
}

type EnvironToolsFinderFunc func(context.Context, int, int, []string, coretools.Filter) (coretools.List, error)

func (e EnvironToolsFinderFunc) FindTools(
	ctx context.Context,
	major,
	minor int,
	streams []string,
	filter coretools.Filter,
) (coretools.List, error) {
	return e.FindTools(ctx, major, minor, streams, filter)
}

func (s *StubService) GetEnvironToolsFinder() EnvironToolsFinderFunc {
	return func(
		ctx context.Context,
		major,
		minor int,
		streams []string,
		filter coretools.Filter,
	) (coretools.List, error) {
		env, err := s.providerForAgentBinaryFinder(ctx)
		if err != nil {
			return nil, err
		}
		ssFetcher := simplestreams.NewSimpleStreams(simplestreams.DefaultDataSourceFactory())
		return envtools.FindTools(ctx, ssFetcher, env, major, minor, streams, filter)
	}
}

This allows us to provide the env tools finder to the common facade code and also allows us to not leak the provider for the service into the facade.

Let me know if you want to chat more about this.

[tlm]

Thanks @ycliuhw,

This is shaping up nicely. It would have been nice to see the implementation seperate from the change in the apiserver as seperate PR's but that is ok.

Are you able to add JUJU-7811 to the PR description as well?

Are you also able to update the description for the PR to redeclare the why now things have changed a bit and why we made this decision?

[tlm]

I understand why the dependencies have been asked for in this way to help support testing and not to produce external changes.

I think however it shouldn't be on the caller to supply this information. Specifically we are trying to make the caller have to have less transitive dependencies and understanding of how the cake is made.

I think if we want to keep this pattern for testing purposes we should do what we do in the workers where this package provides the implementations of the dependencies that can be supplied to the constructor of the stub service.

It allows us to control the dependencies still but not bleed this out to callers.

[tlm]

i think this import alias should be stubdomain or something similar. I don't think that service part comes into it. Service is a type within this package not the package.

[tlm]

This is a big win.