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

feat(cli): bulk export #450

Merged
merged 40 commits into from
Dec 31, 2020
Merged

feat(cli): bulk export #450

merged 40 commits into from
Dec 31, 2020

Conversation

Duologic
Copy link
Member

@Duologic Duologic commented Dec 20, 2020

This ports the functionality for bulk exporting Tanka environments back from our internal CI/CD tooling.

The gist:

  • moved generalized logic from cmd/tk to pkg/tanka
  • add ParseParallel to evaluate envs in parallel
  • tk env list uses ParseParallel
  • tk export --format now takes {{env.<...>}} and fills it with the tanka.dev/Environment object
  • tk export can export multiple environments and find them recursively with --recursive
  • tk export creates a manifest.json file to map files back to an environment
  • tk export can use -l <labelFilters>, similar to tk env list and kubectl

Examples:

# Format based on environment {{env.<...>}}
$ tk export exportDir environments/dev/ --format '{{env.metadata.labels.cluster}}/{{env.spec.namespace}}//{{.kind}}-{{.metadata.name}}'
# Export multiple environments
$ tk export exportDir environments/dev/ environments/qa/
# Recursive export
$ tk export exportDir environments/ --recursive
# Recursive export with labelSelector
$ tk export exportDir environments/ -r -l team=infra

Note: this changes tk export, <exportDir> now comes before the <environment>.

@Duologic Duologic changed the title wip: bulk env actions feat(cli): bulk export Dec 21, 2020
@Duologic Duologic marked this pull request as ready for review December 21, 2020 14:20
@malcolmholmes
Copy link
Contributor

I think the syntax should be to expect a dir to be a single environment, or if -r is specified, recursively look for environments. This gives a clearer expectation of what the command will do - means people must explicitly request the recursive behaviour.

Copy link
Member

@sh0rez sh0rez left a comment

Choose a reason for hiding this comment

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

Took an early look, like the direction this takes.

Left some notes, will do a in-depth review later

cmd/tk/export.go Outdated Show resolved Hide resolved
cmd/tk/export.go Outdated Show resolved Hide resolved
cmd/tk/export.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jvrplmlmn jvrplmlmn left a comment

Choose a reason for hiding this comment

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

Some nits from an initial review, will go later for a second more in-depth round. Looking good 👌

pkg/tanka/environments.go Outdated Show resolved Hide resolved
Comment on lines 130 to 131
data, err := json.MarshalIndent(fileToEnv, "", " ")
if err != nil {
return err
}
path := filepath.Join(to, manifestFile)
if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil {
return fmt.Errorf("creating filepath '%s': %s", filepath.Dir(path), err)
}
if err := ioutil.WriteFile(path, []byte(data), 0644); err != nil {
return err
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

There is some duplicated bits that exist on the cmd/tk/util.go:writeJSON function, if we move that under pkg/... we could reuse it here. Something like:

                path := filepath.Join(to, manifestFile)
		if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil {
			return fmt.Errorf("creating filepath '%s': %s", filepath.Dir(path), err)
		}
                 if err := <import>.WriteJSON(fileToEnv, path); err != nil {
			return err
		}

Copy link
Member Author

Choose a reason for hiding this comment

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

I did a quick grep on WriteFile and found multiple very similar use cases. I propose consolidating that in a separate PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I consolidated this bit in export.go, we can consolidate further in another PR.

pkg/tanka/export.go Outdated Show resolved Hide resolved
pkg/tanka/export.go Outdated Show resolved Hide resolved
pkg/tanka/export.go Outdated Show resolved Hide resolved
pkg/tanka/parse.go Outdated Show resolved Hide resolved
cmd/tk/export.go Outdated Show resolved Hide resolved
pkg/tanka/environments.go Outdated Show resolved Hide resolved
pkg/tanka/errors.go Outdated Show resolved Hide resolved
pkg/tanka/environments.go Outdated Show resolved Hide resolved
pkg/tanka/tanka.go Outdated Show resolved Hide resolved
pkg/tanka/export.go Show resolved Hide resolved
pkg/tanka/parse.go Outdated Show resolved Hide resolved
pkg/tanka/environments.go Outdated Show resolved Hide resolved
@Duologic Duologic force-pushed the duologic/bulk_environment_actions branch from b0f05b3 to 25161b9 Compare December 30, 2020 20:23
@Duologic
Copy link
Member Author

Rebased.

cmd/tk/export.go Outdated
format := cmd.Flags().String("format", "{{.apiVersion}}.{{.kind}}-{{.metadata.name}}", "https://tanka.dev/exporting#filenames")
format := cmd.Flags().String(
"format",
"{{env.spec.namespace}}/{{env.metadata.name}}/{{.apiVersion}}.{{.kind}}-{{.metadata.name}}",
Copy link
Member

Choose a reason for hiding this comment

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

This change possibly breaks existing scripts using tk export. Is it possible to keep the old functionality intact by default?

@Duologic Duologic merged commit d5c878e into master Dec 31, 2020
@Duologic Duologic deleted the duologic/bulk_environment_actions branch December 31, 2020 10:31
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.

4 participants