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

cosa/cli-spec: add envVar CLI support #1

Merged
merged 8 commits into from
Aug 21, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 107 additions & 0 deletions cosa/20200804-cfg-spec.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
# CoreOS Assembler Config Specification
- Subject ID: COSA.20200804
- Superceeds: None
- Conflicts: None

## Summary

The RHCOS development pipelines use a "jobspec" to describe the pipeline runs. This enhancement proposes a YAML-based interface based loosely on the RHCOS jobspec.

## Goals

1. Provide a file-based interface into COSA
1. Prep for breaking up the Pipeline monoliths
1. Provide a migration path for RHCOS pipelines
1. Prepare for pipeline Nirvana (where *COS pipelines can live in peace and harmony)

## Why?

The innovation of the RHCOS JobSpec was it provided a clear seperation of configuration from the code; the logic was conflated. Further the jobspec allowed the logic to be run with different inputs.

To support this, a "COSAspec" is being introduced to provide a CLI file-interface.
Copy link
Member

Choose a reason for hiding this comment

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

Can you include a config name for the YAML file so that we can bikeshed on it? 🎨
I'll throw in a suggestion for cosa-config.yaml.

Also, this proposal doesn't cover where this config needs to live. Is it auto-discovered by cosa? If so, where does it look?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I intentionally left that the where and name of the config out. The rationale is that the COSAspec YAML is something that is brought to COSA. If anything I would run with the idea of it coming from an external repo (e.g. f.c.c or rh.c.c).


## YAML to EnvVars

* For Bash and Pythonic CLIs, each argument will have an envVar added.
* At entry into COSA, the optional COSAspec will be checked
* Values will be parsed and emit as EnvVars by a common entry point.
* The EnvVars will set defaults to the CLI commands

Note: The Mantle code is GoLang based. The use of envVars is supported by the Cobra CLI, but needs further research and consideration. The Mantle code is not considered in this design standard.

## Types

* List values will be joined to comma seperated strings
* Dicts/Map will be flattened with an `_` between levels
* Boolean values should be 0, 1, True, False, true, false.
* Number types are strings and the consuming code is repsonsbile for casting to the type.

Due to the lack of cohesion, the mix of Python, Bash and GoLang, each component will necessarily handle empty and null values.


## Example:

For the `coreos-assembler build` command:
```
build:
force: false
force_nocache: false
skip_prune: false
parent: <string>
tag: <string>
```

Would render the envVars:
```
COSA_BUILD_FORCE=false
COSA_BUILD_FORCE_NOCAHCE=false
COSA_BUILD_SKIP_PRUNE=false
...
```

Then in `cmd-build` the defaults would be:
```
FORCE="${COSA_BUILD_FORCE:-}"
FORCE_IMAGE="${COSA_BUILD_FORCE_IMAGE:-}"
SKIP_PRUNE="${COSA_BUILD_SKIP_PRUNE:-0}"
...
```

## List value example

```
extend:
aws:
regions:
- us-east-1
- us-west-1
```
Would render at `COSA_EXTEND_AWS_REGIONS="us-east-1,us-west-1"`

## Dict example

```
foo:
bar:
baz:
1: true
2: true
```

Would render as `COSA_FOO_BAR_BAZ_1=true` and `COSA_FOO_BAR_BAZ_2=true`

## PR Standard

A thorough review of the COSA code-base, a comparison between the FCOS and RHCOS pipelines shows that each command will need to be handled seperately.

Therefore, for each command updated to modify this interface:
- The prefix `COSA_<COSA COMMAND>` will be used for envVars (e.g. `COSA_EXTEND_AZURE`)
- No calculated values. Commands may not parse the spec file itself.
- One command per PR.
Copy link
Member

Choose a reason for hiding this comment

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

I think to start with, we should keep the scope mostly to the CLI switches we actually really care about for pipelines. Some things are inherently dynamic, e.g. FCOS's cosa build --parent config, and some things are really just developer/instance specific (e.g. cosa build --tag).

Let's add that to the proposal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good idea -- and that's related to why I stated a "PR standard." Dropping a huge PR would be ugly and IMO there are a bunch of commands we don't want this interface for.



Naming convention for commands (in order of precedence):
- `cmd-buildextend-*`: `COSA_EXTEND_*`
- `cmd-build`: `COSA_BUILD_*`
- `cmd-.*`: `COSA_<SUFFIX>_*`
- `.*`: no interface