-
Notifications
You must be signed in to change notification settings - Fork 167
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
Conversation
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. |
There was a problem hiding this 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
There was a problem hiding this 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/export.go
Outdated
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 | ||
} | ||
} |
There was a problem hiding this comment.
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
}
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
setupConfiguration is only used in env.go:envSetCmd() which should fail in case it is missing a spec.json file.
b0f05b3
to
25161b9
Compare
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}}", |
There was a problem hiding this comment.
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?
This ports the functionality for bulk exporting Tanka environments back from our internal CI/CD tooling.
The gist:
cmd/tk
topkg/tanka
ParseParallel
to evaluate envs in paralleltk env list
usesParseParallel
tk export --format
now takes{{env.<...>}}
and fills it with the tanka.dev/Environment objecttk export
can export multiple environments and find them recursively with--recursive
tk export
creates amanifest.json
file to map files back to an environmenttk export
can use-l <labelFilters>
, similar totk env list
andkubectl
Examples:
Note: this changes
tk export
,<exportDir>
now comes before the<environment>
.