Migration source and target implementations by sergenyalcin · Pull Request #124 · crossplane/upjet (original) (raw)

type FileSystemSourceOption func(*FileSystemSource)

// FsWithFileSystem configures the filesystem to use. Used mostly for testing.
func FsWithFileSystem(f afero.Fs) FileSystemSourceOption {

Choose a reason for hiding this comment

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

Are there any fields other than the Fs that we might configure (so that we may prefer to use an afero.Afero as a parameter instead of an afero.Fs in the functional option)?

// we need to add plural appendix to end of kind name
Resource: strings.ToLower(gvk.Kind) + "s",
})
unstructuredList, err := ri.List(context.TODO(), metav1.ListOptions{})

Choose a reason for hiding this comment

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

We had better have an active context here. We should consider revising the Source & Target interfaces.

}

// InitializeDynamicClient returns a dynamic client
func InitializeDynamicClient(kubeconfigPath string) (dynamic.Interface, error) {

Choose a reason for hiding this comment

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

We consider this as a utility to construct dynamic clients, is this correct?

Choose a reason for hiding this comment

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

Yes, this is correct!

muvaf mentioned this pull request

Nov 3, 2022

All committers have signed the CLA.

Choose a reason for hiding this comment

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

Thanks @sergenyalcin for the implementations. I think we are ready to merge them and do some further iterations as we gain more experience on the migration use cases and corner cases.

Because you are busy with support rotation, I can take care of rebasing the branch and merging it as is. Also the PR #119 that contains the base commit of this PR's feature branch has been merged.

Signed-off-by: Sergen Yalçın yalcinsergen97@gmail.com

Hi @sergenyalcin,
I have rebased your PR. I only did the following changes in your feature branch:

Moved test resources awsvpc.yaml and resourcegroup.yaml from testdata to testdata/source.

Added the following error check in the test file pkg/migration/filesystem_test.go:

      fs, err := NewFileSystemSource("testdata/source", FsWithFileSystem(files))
      if err != nil {
          t.Fatalf("Failed to initialize a new FileSystemSource: %v", err)
      }

Updated references to testdata with testdata/source.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})