pkg/asset/installconfig: Add _CI_ONLY_STAY_AWAY_OPENSHIFT_INSTALL_AWS_USER_TAGS #364
Conversation
openshift-ci-robot
added
approved
Instead of as an array of strings. This makes it much easier to add control over additional properties, because the consuming logic remains "just unpack the JSON into the appropriate structure".
wking
force-pushed
the
platform-asset-state-as-json
branch
from
adf12e9
to
931cfa3
Compare
|
@wking This is definitely useful. |
pkg/asset/installconfig/platform.go
Outdated
| @@ -162,6 +163,12 @@ func (a *Platform) awsPlatform() (*asset.State, error) { | |||
| } | |||
| platform.Region = string(region.Contents[0].Data) | |||
|
|
|||
| if value, ok := os.LookupEnv("OPENSHIFT_INSTALL_AWS_USER_TAGS"); ok { | |||
There was a problem hiding this comment.
We should keep track of these internal MAGIC envs. This might get lost.
There was a problem hiding this comment.
We should keep track of these internal MAGIC envs.
Does the rename address this concern?
…_USER_TAGS With a JSON string containing the intended values. This is not how we're going to expose this long-term, so I'm using an embarrassing name and not documenting the enviroment variable. But I want this so we can get back to tagging expirationData in CI, without waiting for asset state <-> disk (de)serialization.
wking
changed the title
wking
force-pushed
the
platform-asset-state-as-json
branch
from
931cfa3
to
98a3531
Compare
|
I've pushed 931cfa3 -> 98a3531 to give this a more embarassing name, so it's more obvious for folks grepping the source that this is not going to be the stable way to inject this information (the stable approach is going to be creating your own install-config YAML and feeding that into |
|
/approve |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: abhinavdahiya, crawford, wking The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
We'd dropped this in 3f2f01c (ci-operator/config/openshift/installer/master: Move to openshift-install, 2018-09-26, openshift#1677). But since openshift/installer@98a35316 (pkg/asset/installconfig: Add _CI_ONLY_STAY_AWAY_OPENSHIFT_INSTALL_AWS_USER_TAGS, 2018-09-28, openshift/installer#364), the installer supports setting it again, so we can add it back. As the name suggests, this variable is not a stable API. Soon the new installer will be able to load the configuration from YAML (again), but we can keep adjusting as the installer evolves.
With a JSON string containing the intended values.
This is probably not how we're going to expose this long-term, so I'm not documenting the enviroment variable yet. But I want this so we can get back to tagging expirationData in CI, without waiting for asset state <-> disk (de)serialization.
I've also adjusted installconfig to pass platforms as JSON instead of as an array of strings. This makes it much easier to add control over additional properties, because the consuming logic remains "just unpack the JSON into the appropriate structure". This may conflict with #344, but it's a small enough change that rebasing either way should be easy. CC @staebler.