Simplify adding a external provider

[tumberino]

With this change it will be easier to create a provider without being merged into our existing code. Instead they can use the existing code as a library to enable their provider.

They would then just need to create their own main.go or copy ours as a starting point. And use the import style to include their own provider logic which can exist externally to our code.

Example Provider

After this change the following will be possible

.
├── main.go
├── pkg
│   └── provider
│       └── example
│           ├── manager.go
│           ├── provider.go
│           └── types.go
└── providers.go

main.go > This is will be the same as upstream main

manager.go

package example

import (
	"flag"
	"github.com/confidential-containers/cloud-api-adaptor/pkg/adaptor/cloud"
)

var exampleConfig Config
type Manager struct{}
func init() {
	cloud.AddCloud("example", &Manager{})
}
func (*Manager) ParseCmd(flags *flag.FlagSet) {}
func (*Manager) LoadEnv() {}
func (*Manager) NewProvider() (cloud.Provider, error) {
	return NewProvider(&exampleConfig)
}

provider.go

package example

import (
	"context"
	"github.com/confidential-containers/cloud-api-adaptor/pkg/adaptor/cloud"
	"github.com/confidential-containers/cloud-api-adaptor/pkg/util/cloudinit"
)

type exampleProvider struct {
}
func NewProvider(config *Config) (cloud.Provider, error) {
	return &exampleProvider{}, nil
}
func (p *exampleProvider) CreateInstance(ctx context.Context, podName, sandboxID string, cloudConfig cloudinit.CloudConfigGenerator, spec cloud.InstanceTypeSpec) (*cloud.Instance, error) {
	return nil, nil
}
func (p *exampleProvider) DeleteInstance(ctx context.Context, instanceID string) error {
	return nil
}
func (p *exampleProvider) Teardown() error {
	return nil
}
func (p *exampleProvider) ConfigVerifier() error {
	return nil
}

types.go

package example

type Config struct {
}

[tumberino]

Had to drop 9d0a64a because need these changes merged and then update the controller with these changes plus updating the version of caa it references

[bpradipt]

@jtumber-ibm I believe with the changes in this PR, CAA project can be used as a library ?

[tumberino]

@jtumber-ibm I believe with the changes in this PR, CAA project can be used as a library ?

Yes, that is the plan

[snir911]

Would it make sense to update also docs/addnewprovider.md as part of this PR?
LGTM overall, peerpods-ctrl will require update after merge.

BTW worth mentioning that moving (build) tagged code around made compiling issues before, however, LGTM this time

[yoheiueda]

@jtumber-ibm Thank you very much for this PR. I didn't aware of the problem that prevents us from using CAA as a library.

I have one question about the root cause of the problem.

The problem is that each provider code needs to update the cloudTable map in the cloudmgr package, and it is not a public variable. Right? If so, isn't adding the public function AddCloud to the original cloudmgr package enough?

As @snir911 mentioned, peerpods-ctrl also uses the package. The cloudmgr pacakge was originally in cmd/cloud-api-adaptor, and was later moved to pkg/cloud, so that we can share it with peerpods-ctrl.

If we move files like aws.go and ibmcloud.go back to cmd/cloud-api-adaptor, I think we also need to copy the same files to the peerpod-ctrl directory. I think it is somewhat redundant.

WDYT?

[tumberino]

@yoheiueda see 9d0a64a for how to include the providers in the peerpod-ctrl.

By moving the logic out of that shared package it also means the provider specific dependencies are also separated out. In other words leaving it as is but making the public function change when someone wants to use our project their go.mod/go.sum will include the provider specific dependencies for all our current providers even when you don't want to use them.

[yoheiueda]

@jtumber-ibm Thank you for the explanation.

I got the point. We can't rely on build tags when we wan to exclude unnecessary dependencies in go.mod/go.sum. I am not aware of this point.

[yoheiueda]

LGTM

[bpradipt]

/lgtm
@tumberino did you get a chance to test the built caa binary with libvirt and ibmcloud ?

[tumberino]

/lgtm @tumberino did you get a chance to test the built caa binary with libvirt and ibmcloud ?

Only tested with ibmcloud so far, do we want to take this approach now and something like #1583 later?

[bpradipt]

/lgtm @tumberino did you get a chance to test the built caa binary with libvirt and ibmcloud ?

Only tested with ibmcloud so far, do we want to take this approach now and something like #1583 later?

I am ok either way, ie having this one now and 1583 later. But if 1583 is ready and supersedes this PR then better to go with 1583. I haven't looked at 1583 yet.

[tumberino]

/lgtm @tumberino did you get a chance to test the built caa binary with libvirt and ibmcloud ?

Only tested with ibmcloud so far, do we want to take this approach now and something like #1583 later?

I am ok either way, ie having this one now and 1583 later. But if 1583 is ready and supersedes this PR then better to go with 1583. I haven't looked at 1583 yet.

#1583 is a lot lot larger change and therefore has more risk than this one. So I think we go with this one first whilst we wait for more community input on the approach to remove the cyclic dependency issue

[katexochen]

This has broken some link: https://github.com/confidential-containers/cloud-api-adaptor/actions/runs/7078374683/job/19263779058