config: add config subcommand #2405

s-urbaniak · 2016-04-11T08:39:47Z

This command adds the config subcommand.

TODOs:

Fixes #2368

s-urbaniak · 2016-04-11T10:33:50Z

@jonboulle @alban @krnowak PTAL; stage1 is not ready yet, learning that part right now.

s-urbaniak · 2016-04-11T10:36:20Z

I suggest implementing stage1 in a different PR in case this is lgtm.

jonboulle · 2016-04-11T12:30:13Z

Documentation/subcommands/config.md

+```
+$ rkt config
+{
+  "stage0": [


OOB notes:

define top-level struct, as this is new

explain why we are printing each entry separately (even if the input config was deduplicated)

explain oauth/basic mutual incompatibility and override semantics

also, please capture everything from the design doc in here

Addressed OOB notes. I could not reproduce the oauth/basic mutual incompatibility, I mixed up the already documented override semantics of system/user directories.

jonboulle · 2016-04-11T12:31:30Z

/cc @josephschorr

alban · 2016-04-11T14:19:54Z

Documentation/subcommands/config.md

+}
+```
+
+And the follwong user configuration:


s/follwong/following

s-urbaniak · 2016-04-12T09:38:33Z

@alban @krnowak @jonboulle PTAL

krnowak · 2016-04-12T10:26:17Z

Documentation/subcommands/config.md

+
+The entries "stage0" and "stage1" refer to stage-specific configuration. The generated output are valid configuration entries as specified in the configuration documentation.
+
+The "stage0" entry contains entries of rktKind "auth", "dockerAuth", and "paths". Note that the `config` subcommand will output separate entries per "auth" domain and separate entries per "dockerAuth" registry. While it is possible to specify an array of strings in the input configuration rkt internally merges configuration state from different directories potentially creating multiple entries.


I'll make my plans about configuration clear here.

stage0 configuration is the configuration read by the rkt binary. So, rkt needs to know the credentials for fetching images (auth and dockerAuth kinds), needs to know where to store the fetched images (paths kind) and needs to know which stage1 image should be used by default and where to look for it (stage1 kind).

stage1 configuration will keep the configuration read by the binaries inside stage1 image (in our case init, enter and so on). So in this entry, we would put networking configuration and other stage1 specific settings.

So, in the current PR, stage1 entry should be empty.

Also, about the multiple auth entries - that's an implementation detail. We may change the code later to be smarter and merge the blocks with the same credentials. So I would keep quiet about it to avoid giving an impression that "domains" key is always a table with only one element.

Hmm .. not sure about this, @jonboulle suggested to be transparent about it but you have a point, it's an implementation detail.

It's partly an implementation detail, but it's also unintuitive why there's an asymmetry if I feed in a config with multiple entries and get back multiple stanzas with single entries. I think we can strike a balance explaining that they are semantically identical.

The asymmetry is only because of the implementation. Anyway, this can stay for now, but I'd like to have a warning or something that one should not assume that it will stay like that (means - one entry for each domain). "domains" key is an array for a reason.

I addressed this by adding a statement that the current merged/non-merged output behavior is subject to change.

s-urbaniak · 2016-04-13T12:22:00Z

@alban @krnowak @jonboulle PTAL; if looks good, I'll squash being ready for the release.

jonboulle · 2016-04-13T12:36:40Z

Documentation/subcommands/config.md

@@ -0,0 +1,125 @@
+# rkt config


Paging @joshix for doc review.

jonboulle · 2016-04-13T12:41:14Z

What's with the test failure? https://semaphoreci.com/coreos/rkt/branches/pull-request-2405/builds/17

jonboulle · 2016-04-13T12:42:18Z

Since this becomes API, I'd like to see a test that this outputs what we expect (and is consumable again, if that's what we're promising), rather than just a successful marshalling. In the interests of getting this in 1.4.0 I'm okay with that being a follow-up, but we need some guarantees to avoid future breakage.

s-urbaniak · 2016-04-13T13:04:10Z

@jonboulle issue already created, xref'ing here: #2410

josephschorr · 2016-04-13T17:52:22Z

Looks good from a usage perspective; excited to add support.

joshix · 2016-04-13T18:04:33Z

Documentation/subcommands/config.md

@@ -0,0 +1,125 @@
+# rkt config
+
+This command prints the rkt configuration per stage in JSON format.


The config subcommand prints the configuration of each rkt stage in JSON on the standard output.

(have to break reviewing this until later today)

fixed, thanks @joshix!

krnowak · 2016-04-14T12:36:29Z

rkt/config.go

+	cmdRkt.AddCommand(cmdConfig)
+}
+
+func runConfig(cmd *cobra.Command, args []string) (exit int) {


I guess that (exit int) is a copy-pasta from other commands, you don't use it here, so just simplify it to int.

krnowak · 2016-04-14T12:39:10Z

Two final nits, otherwise LFAD.

This adds the config subcommand. Currently only stage0 configuration is supported. Fixes rkt#2368

s-urbaniak force-pushed the config branch 5 times, most recently from 8e5e987 to b9c93ed Compare April 11, 2016 10:27

s-urbaniak changed the title ~~WIP config: add config subcommand~~ config: add config subcommand Apr 11, 2016

jonboulle reviewed Apr 11, 2016
View reviewed changes

alban reviewed Apr 11, 2016
View reviewed changes

Documentation/subcommands/config.md

}

```

And the follwong user configuration:

Copy link

Member

alban Apr 11, 2016

Choose a reason for hiding this comment

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

s/follwong/following

s-urbaniak mentioned this pull request Apr 12, 2016

rkt pull credential support quay/quayctl#37

Closed

jonboulle added needs/review priority/P1 area/configuration kind/enhancement area/usability labels Apr 12, 2016

jonboulle added this to the v1.4.0 milestone Apr 12, 2016

krnowak reviewed Apr 12, 2016
View reviewed changes

s-urbaniak force-pushed the config branch 2 times, most recently from e699eeb to 76d4023 Compare April 12, 2016 10:57

jonboulle reviewed Apr 13, 2016
View reviewed changes

Documentation/subcommands/config.md

@@ -0,0 +1,125 @@

# rkt config

Copy link

Contributor

jonboulle Apr 13, 2016

Choose a reason for hiding this comment

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

Paging @joshix for doc review.

s-urbaniak force-pushed the config branch from 15021a9 to 6e7c18b Compare April 13, 2016 14:21

s-urbaniak force-pushed the config branch from 6e7c18b to 56cf426 Compare April 13, 2016 14:21

joshix reviewed Apr 13, 2016
View reviewed changes

s-urbaniak force-pushed the config branch 3 times, most recently from a55d1f5 to 3fab112 Compare April 13, 2016 18:41

krnowak reviewed Apr 14, 2016
View reviewed changes

krnowak added needs/some-changes and removed needs/review labels Apr 14, 2016

config: add config subcommand

c26f621

This adds the config subcommand. Currently only stage0 configuration is supported. Fixes rkt#2368

s-urbaniak force-pushed the config branch from 3fab112 to c26f621 Compare April 14, 2016 13:01

iaguis added the reviewed/lgtm label Apr 14, 2016

iaguis merged commit ed0d6dc into rkt:master Apr 14, 2016

alban mentioned this pull request Apr 15, 2016

Release v1.4.0 #2431

Merged

s-urbaniak deleted the config branch April 20, 2016 06:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config: add config subcommand #2405

config: add config subcommand #2405


		The entries "stage0" and "stage1" refer to stage-specific configuration. The generated output are valid configuration entries as specified in the configuration documentation.

		The "stage0" entry contains entries of rktKind "auth", "dockerAuth", and "paths". Note that the `config` subcommand will output separate entries per "auth" domain and separate entries per "dockerAuth" registry. While it is possible to specify an array of strings in the input configuration rkt internally merges configuration state from different directories potentially creating multiple entries.

		@@ -0,0 +1,125 @@
		# rkt config

		This command prints the rkt configuration per stage in JSON format.

config: add config subcommand #2405

config: add config subcommand #2405

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment