Add support for ignite configuration file #601
Conversation
7902818 to
4b897ce
Compare
d6d08d7 to
b5ab200
Compare
|
I'd add this to a new v1alpha3 API version, in order to not break v1alpha2 compatibility. ref: #355 |
b5ab200 to
59ac3bc
Compare
89cbaeb to
c05c2cc
Compare
|
Rebased on v1alpha3 API. |
cmd/ignite/cmd/root.go
Outdated
| @@ -115,9 +153,27 @@ func addGlobalFlags(fs *pflag.FlagSet) { | |||
| logflag.LogLevelFlagVar(fs, &logLevel) | |||
| runtimeflag.RuntimeVar(fs, &providers.RuntimeName) | |||
| networkflag.NetworkPluginVar(fs, &providers.NetworkPluginName) | |||
| fs.StringVar(&configPath, "ignite-config", "", "Ignite configuration path") | |||
There was a problem hiding this comment.
More descriptive note on what this flag does
There was a problem hiding this comment.
I've added a document about this in docs/ignite-configuration.md. I hope this will make it more clear.
Should this still be more descriptive?
| } | ||
| 8000 | } | |
|
|
||
| if configFilePath != "" { |
There was a problem hiding this comment.
Also handle the config for ignited?
There was a problem hiding this comment.
Maybe in a follow-up PR?
This adds a new API type `Configuration` which is used to define the
default configurations of ignite. A Configuration object can be defined
in a config file as:
```yaml
apiVersion: ignite.weave.works/v1alpha3
kind: Configuration
metadata:
name: test-config
spec:
runtime: containerd
networkPlugin: cni
vmDefaults:
memory: "2GB"
diskSize: "3GB"
cpus: 2
sandbox:
oci: weaveworks/ignite:dev
kernel:
oci: weaveworks/ignite-kernel:4.19.47
ssh: true
```
By default, ignite looks for the config file at /etc/ignite/config.yaml.
If this file is found, ignite uses it to configure the ignite defaults.
`--ignite-config` flag c
8000
an be used to explicitly pass a configuration
file.
0f9bea6 to
241c223
Compare
- Flag overrides are applied on VM even if a VM config file is provided.
c3fa727 to
96dc1f6
Compare
96dc1f6 to
4679153
Compare
| err: true, | ||
| }, | ||
| { | ||
| name: "with sandbox image", |
There was a problem hiding this comment.
This is already tested in e2e so it's fine to exclude it here (tricky to implement).
| wantSSH: api.SSH{ | ||
| Generate: true, | ||
| PublicKey: "some-pub-key", | ||
| }, | ||
| }, | ||
| { | ||
| name: "with no VM name and --require-name flag set", |
There was a problem hiding this comment.
This was tested twice, check https://github.com/weaveworks/ignite/pull/601/files#diff-d0bf10b7da8bd0e055593597d507f004R306
| } | ||
|
|
||
| for _, rt := range tests { | ||
| t.Run(rt.name, func(t *testing.T) { | ||
| err := rt.createFlag.constructVMFromCLI(rt.args) | ||
| vm := rt.createFlag.VM | ||
| fs := flag.NewFlagSet("test", flag.ExitOnError) |
There was a problem hiding this comment.
This does not test flag validity since that would need the flag set to be populated (needs additional logic), but the flag handling will need to be refactored at some point anyway so this is fine.
| if err != nil { | ||
| t.Fatalf("failed to create storage for ignite: %v", err) | ||
| } | ||
| defer os.RemoveAll(dir) |
There was a problem hiding this comment.
Catch this error with util.DeferErr (or assert) (will be done in a separate PR)
| } | ||
| defer os.RemoveAll(dir) | ||
|
|
||
| storage := cache.NewCache( |
There was a problem hiding this comment.
This should use Ignite's configured providers to have consistency between the tests and built binary, can be done in a separate PR.
make api-docs
This adds a new API type
Configurationwhich is used to define thedefault configurations of ignite. A Configuration object can be defined
in a config file as:
By default, ignite looks for the config file at /etc/ignite/config.yaml.
If this file is found, ignite uses it to configure the ignite defaults.
--ignite-configflag can be used to explicitly pass a configurationfile.
Default config:
With global config:
docs/ignite-configuration.mdwith examples of the ignite configuration.Related to #315