Skip to content

Commit

Permalink
assets: add tests for validating asset fetching of targets
Browse files Browse the repository at this point in the history
This adds tests around creating the targeted assets. The tests validate
the following.

1) An asset that is generated and then fetched again from a new store
has equivalent state.
2) A targeted asset restored from the state file and from on-disk has
equivalent state.

To keep from duplicating the code defining the assets including in the
targets, the definitions have been moved to pkg/asset/targets/targets.go.

Fixes https://jira.coreos.com/browse/CORS-940
  • Loading branch information
staebler committed Feb 1, 2019
1 parent c6f6449 commit 75ab106
Show file tree
Hide file tree
Showing 7 changed files with 196 additions and 46 deletions.
19 changes: 6 additions & 13 deletions cmd/openshift-install/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,8 @@ import (
configclient "github.com/openshift/client-go/config/clientset/versioned"
routeclient "github.com/openshift/client-go/route/clientset/versioned"
"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/asset/cluster"
"github.com/openshift/installer/pkg/asset/ignition/bootstrap"
"github.com/openshift/installer/pkg/asset/ignition/machine"
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/asset/kubeconfig"
"github.com/openshift/installer/pkg/asset/manifests"
assetstore "github.com/openshift/installer/pkg/asset/store"
"github.com/openshift/installer/pkg/asset/templates"
"github.com/openshift/installer/pkg/asset/tls"
targetassets "github.com/openshift/installer/pkg/asset/targets"
destroybootstrap "github.com/openshift/installer/pkg/destroy/bootstrap"
cov1helpers "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers"
)
Expand All @@ -56,7 +49,7 @@ var (
// FIXME: add longer descriptions for our commands with examples for better UX.
// Long: "",
},
assets: []asset.WritableAsset{&installconfig.InstallConfig{}},
assets: targetassets.InstallConfig,
}

manifestsTarget = target{
Expand All @@ -67,7 +60,7 @@ var (
// FIXME: add longer descriptions for our commands with examples for better UX.
// Long: "",
},
assets: []asset.WritableAsset{&manifests.Manifests{}, &manifests.Openshift{}},
assets: targetassets.Manifests,
}

manifestTemplatesTarget = target{
Expand All @@ -77,7 +70,7 @@ var (
Short: "Generates the unrendered Kubernetes manifest templates",
Long: "",
},
assets: templates.Templates,
assets: targetassets.ManifestTemplates,
}

ignitionConfigsTarget = target{
Expand All @@ -88,7 +81,7 @@ var (
// FIXME: add longer descriptions for our commands with examples for better UX.
// Long: "",
},
assets: []asset.WritableAsset{&bootstrap.Bootstrap{}, &machine.Master{}, &machine.Worker{}, &kubeconfig.Admin{}, &cluster.Metadata{}},
assets: targetassets.IgnitionConfigs,
}

clusterTarget = target{
Expand Down Expand Up @@ -129,7 +122,7 @@ var (
}
},
},
assets: []asset.WritableAsset{&cluster.TerraformVariables{}, &kubeconfig.Admin{}, &tls.JournalCertKey{}, &cluster.Metadata{}, &cluster.Cluster{}},
assets: targetassets.Cluster,
}

targets = []target{installConfigTarget, manifestTemplatesTarget, manifestsTarget, ignitionConfigsTarget, clusterTarget}
Expand Down
4 changes: 4 additions & 0 deletions pkg/asset/machines/master.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ func (m *Master) Generate(dependencies asset.Parents) error {
}
m.MachinesRaw = raw
case nonetypes.Name:
// This is needed to ensure that roundtrip generate-load tests pass when
// comparing this value. Otherwise, generate will use a nil value while
// load will use an empty byte slice.
m.MachinesRaw = []byte{}
case openstacktypes.Name:
mpool := defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName)
mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform)
Expand Down
4 changes: 4 additions & 0 deletions pkg/asset/machines/worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ func (w *Worker) Generate(dependencies asset.Parents) error {
}
w.MachineSetRaw = raw
case nonetypes.Name:
// This is needed to ensure that roundtrip generate-load tests pass when
// comparing this value. Otherwise, generate will use a nil value while
// load will use an empty byte slice.
w.MachineSetRaw = []byte{}
case openstacktypes.Name:
mpool := defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName)
mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform)
Expand Down
113 changes: 113 additions & 0 deletions pkg/asset/store/assetcreate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
package store

import (
"io/ioutil"
"os"
"path/filepath"
"reflect"
"testing"

"github.com/stretchr/testify/assert"

"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/asset/targets"
)

const userProvidedAssets = `{
"*installconfig.baseDomain": {
"BaseDomain": "test-domain"
},
"*installconfig.clusterID": {
"ClusterID": "test-cluster-id"
},
"*installconfig.clusterName": {
"ClusterName": "test-cluster"
},
"*installconfig.platform": {
"none": {}
},
"*installconfig.pullSecret": {
"PullSecret": "{\"auths\": {\"example.com\": {\"auth\": \"test-auth\"}}}\n"
},
"*installconfig.sshPublicKey": {}
}`

func TestCreatedAssetsAreNotDirty(t *testing.T) {
cases := []struct {
name string
targets []asset.WritableAsset
}{
{
name: "install config",
targets: targets.InstallConfig,
},
{
name: "manifest templates",
targets: targets.ManifestTemplates,
},
{
name: "manifests",
targets: targets.Manifests,
},
{
name: "ignition configs",
targets: targets.IgnitionConfigs,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
tempDir, err := ioutil.TempDir("", "TestCreatedAssetsAreNotDirty")
if err != nil {
t.Fatalf("could not create the temp dir: %v", err)
}
defer os.RemoveAll(tempDir)

if err := ioutil.WriteFile(filepath.Join(tempDir, stateFileName), []byte(userProvidedAssets), 0666); err != nil {
t.Fatalf("could not write the state file: %v", err)
}

assetStore, err := newStore(tempDir)
if err != nil {
t.Fatalf("failed to create asset store: %v", err)
}

for _, a := range tc.targets {
if err := assetStore.Fetch(a); err != nil {
t.Fatalf("failed to fetch %q: %v", a.Name(), err)
}

if err := asset.PersistToFile(a, tempDir); err != nil {
t.Fatalf("failed to write asset %q to disk: %v", a.Name(), err)
}
}

newAssetStore, err := newStore(tempDir)
if err != nil {
t.Fatalf("failed to create new asset store: %v", err)
}

for _, a := range tc.targets {
newAsset := reflect.New(reflect.TypeOf(a).Elem()).Interface().(asset.WritableAsset)
if err := newAssetStore.Fetch(newAsset); err != nil {
t.Fatalf("failed to fetch %q in new store: %v", a.Name(), err)
}
assetState := newAssetStore.assets[reflect.TypeOf(a)]
// Make an exception for metadata. It's files are read-only.
if a.Name() != "Metadata" {
assert.Truef(t, assetState.presentOnDisk, "asset %q was not found on disk", a.Name())
}
}

assert.Equal(t, len(assetStore.assets), len(newAssetStore.assets), "new asset store does not have the same number of assets as original")

for _, a := range newAssetStore.assets {
originalAssetState, ok := assetStore.assets[reflect.TypeOf(a.asset)]
if !ok {
t.Fatalf("asset %q not found in original store", a.asset.Name())
}
assert.Equalf(t, originalAssetState.asset, a.asset, "fetched and generated asset %q are not equal", a.asset.Name())
assert.Equalf(t, stateFileSource, a.source, "asset %q was not fetched from the state file", a.asset.Name())
}
})
}
}
67 changes: 67 additions & 0 deletions pkg/asset/targets/targets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
package targets

import (
"github.com/openshift/installer/pkg/asset"
"github.com/openshift/installer/pkg/asset/cluster"
"github.com/openshift/installer/pkg/asset/ignition/bootstrap"
"github.com/openshift/installer/pkg/asset/ignition/machine"
"github.com/openshift/installer/pkg/asset/installconfig"
"github.com/openshift/installer/pkg/asset/kubeconfig"
"github.com/openshift/installer/pkg/asset/manifests"
"github.com/openshift/installer/pkg/asset/templates/content/bootkube"
"github.com/openshift/installer/pkg/asset/templates/content/openshift"
"github.com/openshift/installer/pkg/asset/tls"
)

var (
// InstallConfig are the install-config targeted assets.
InstallConfig = []asset.WritableAsset{
&installconfig.InstallConfig{},
}

// Manifests are the manifests targeted assets.
Manifests = []asset.WritableAsset{
&manifests.Manifests{},
&manifests.Openshift{},
}

// ManifestTemplates are the manifest-templates targeted assets.
ManifestTemplates = []asset.WritableAsset{
&bootkube.KubeCloudConfig{},
&bootkube.MachineConfigServerTLSSecret{},
&bootkube.OpenshiftServiceCertSignerSecret{},
&bootkube.Pull{},
&bootkube.CVOOverrides{},
&bootkube.HostEtcdServiceEndpointsKubeSystem{},
&bootkube.KubeSystemConfigmapEtcdServingCA{},
&bootkube.KubeSystemConfigmapRootCA{},
&bootkube.KubeSystemSecretEtcdClient{},
&bootkube.OpenshiftMachineConfigOperator{},
&bootkube.OpenshiftClusterAPINamespace{},
&bootkube.OpenshiftServiceCertSignerNamespace{},
&bootkube.EtcdServiceKubeSystem{},
&bootkube.HostEtcdServiceKubeSystem{},
&openshift.BindingDiscovery{},
&openshift.CloudCredsSecret{},
&openshift.KubeadminPasswordSecret{},
&openshift.RoleCloudCredsSecretReader{},
}

// IgnitionConfigs are the ignition-configs targeted assets.
IgnitionConfigs = []asset.WritableAsset{
&kubeconfig.Admin{},
&machine.Master{},
&machine.Worker{},
&bootstrap.Bootstrap{},
&cluster.Metadata{},
}

// Cluster are the cluster targeted assets.
Cluster = []asset.WritableAsset{
&cluster.TerraformVariables{},
&kubeconfig.Admin{},
&tls.JournalCertKey{},
&cluster.Cluster{},
&cluster.Metadata{},
}
)
2 changes: 2 additions & 0 deletions pkg/asset/templates/doc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Package templates deals with creating template assets that will be used by other assets
package templates
33 changes: 0 additions & 33 deletions pkg/asset/templates/templates.go

This file was deleted.

0 comments on commit 75ab106

Please sign in to comment.