types/validation: add version to install-config

[crawford]

The install-config needs to be versioned in order to allow the installer
to make changes, breaking or otherwise. Since the install-config is
already a valid Kubernetes object, it adopts the same versioning scheme.

cc @dgoodwin @wking @abhinavdahiya @smarterclayton

[wking]

has some unit-test errors, but otherwise looks good to me.

[wking]

Do we want to SemVer this to make it easier to see feature additions? Or will we not bump on feature additions and only bunp this for backwards-incompatible changes?

[crawford]

I was originally going to use semver, but then decided with K8S versioning. This was just done for continuity with the rest of the ecosystem. If we were to adopt semver instead, would you still want to use the same field, or should we create a different version field?

[wking]

I was originally going to use semver, but then decided with K8S versioning.

Either is fine with me, I just want to know what the plan is ;).

This was just done for continuity with the rest of the ecosystem. If we were to adopt semver instead, would you still want to use the same field, or should we create a different version field?

I'm fine using this field for whatever version strings we pick.

[wking]

You'll need to patch this into the openshift/release templates to avoid:

level=fatal msg="failed to fetch Terraform Variables: failed to load asset \"Install Config\": invalid \"install-config.yaml\" file: apiVersion: Required value: install-config version required"

See openshift/release#2506 for a similar adjustment. Or update this PR to assume unset means whatever we call the initial version.

[crawford]

Requires openshift/release#2559 (and any other changes we want to make to the install config).

/hold

[crawford]

Just want to confirm that CI change.

/retest

[abhinavdahiya]

/hold cancel
/lgtm
/retest

[crawford]

/hold

We need to wait for the other PRs which change the format of the install config to merge first.

[crawford]

All of the other changes to the install config have landed. I'm going to let this one through now.

/hold cancel

[wking]

Hrm, this will need a rebase, but maybe just for unit tests, so should be fine post-freeze.

[crawford]

The following patch was needed once ec92355 was merged:

diff --git a/pkg/asset/installconfig/installconfig_test.go b/pkg/asset/installconfig/installconfig_test.go
index 77736f562..1874e8240 100644
--- a/pkg/asset/installconfig/installconfig_test.go
+++ b/pkg/asset/installconfig/installconfig_test.go
@@ -57,6 +57,9 @@ func TestInstallConfigGenerate_FillsInDefaults(t *testing.T) {
                t.Errorf("unexpected error generating install config: %v", err)
        }
        expected := &types.InstallConfig{
+               TypeMeta: metav1.TypeMeta{
+                       APIVersion: "v1beta1",
+               },
                ObjectMeta: metav1.ObjectMeta{
                        Name: "test-cluster",
                },
@@ -102,6 +105,7 @@ func TestInstallConfigLoad(t *testing.T) {
                {
                        name: "valid InstallConfig",
                        data: `
+apiVersion: v1beta1
 metadata:
   name: test-cluster
 baseDomain: test-domain
@@ -112,6 +116,9 @@ pullSecret: "{\"auths\":{\"example.com\":{\"auth\":\"authorization value\"}}}"
 `,
                        expectedFound: true,
                        expectedConfig: &types.InstallConfig{
+                               TypeMeta: metav1.TypeMeta{
+                                       APIVersion: "v1beta1",
+                               },
                                ObjectMeta: metav1.ObjectMeta{
                                        Name: "test-cluster",
                                },

[wking]

/lgtm

[openshift-ci-robot]

[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

Details

Needs approval from an approver in each of these files:

• OWNERS [abhinavdahiya,crawford,wking]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[sttts]

Shouldn't we conform with Kubernetes api versions and put this into an api group, like installer.openshift.io/v1beta1 ?