Skip to content

Commit da6d45b

Browse files
openshift-merge-robotopenshift-merge-robot
authored
Merge pull request #890 from staebler/asset_loading_tests
 assets: add tests for validating asset fetching of targets
2 parents f19a713 + 17a3965 commit da6d45b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+1465
-1334
lines changed

‎cmd/openshift-install/create.go

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,8 @@ import (
2626
configclient "github.com/openshift/client-go/config/clientset/versioned"
2727
routeclient "github.com/openshift/client-go/route/clientset/versioned"
2828
"github.com/openshift/installer/pkg/asset"
29-
"github.com/openshift/installer/pkg/asset/cluster"
30-
"github.com/openshift/installer/pkg/asset/ignition/bootstrap"
31-
"github.com/openshift/installer/pkg/asset/ignition/machine"
32-
"github.com/openshift/installer/pkg/asset/installconfig"
33-
"github.com/openshift/installer/pkg/asset/kubeconfig"
34-
"github.com/openshift/installer/pkg/asset/manifests"
35-
"github.com/openshift/installer/pkg/asset/templates"
36-
"github.com/openshift/installer/pkg/asset/tls"
29+
assetstore "github.com/openshift/installer/pkg/asset/store"
30+
targetassets "github.com/openshift/installer/pkg/asset/targets"
3731
destroybootstrap "github.com/openshift/installer/pkg/destroy/bootstrap"
3832
cov1helpers "github.com/openshift/library-go/pkg/config/clusteroperator/v1helpers"
3933
)
@@ -55,7 +49,7 @@ var (
5549
// FIXME: add longer descriptions for our commands with examples for better UX.
5650
// Long: "",
5751
},
58-
assets: []asset.WritableAsset{&installconfig.InstallConfig{}},
52+
assets: targetassets.InstallConfig,
5953
}
6054

6155
manifestsTarget = target{
@@ -66,7 +60,7 @@ var (
6660
// FIXME: add longer descriptions for our commands with examples for better UX.
6761
// Long: "",
6862
},
69-
assets: []asset.WritableAsset{&manifests.Manifests{}, &manifests.Openshift{}},
63+
assets: targetassets.Manifests,
7064
}
7165

7266
manifestTemplatesTarget = target{
@@ -76,7 +70,7 @@ var (
7670
Short: "Generates the unrendered Kubernetes manifest templates",
7771
Long: "",
7872
},
79-
assets: []asset.WritableAsset{&templates.Templates{}},
73+
assets: targetassets.ManifestTemplates,
8074
}
8175

8276
ignitionConfigsTarget = target{
@@ -87,7 +81,7 @@ var (
8781
// FIXME: add longer descriptions for our commands with examples for better UX.
8882
// Long: "",
8983
},
90-
assets: []asset.WritableAsset{&bootstrap.Bootstrap{}, &machine.Master{}, &machine.Worker{}, &kubeconfig.Admin{}, &cluster.Metadata{}},
84+
assets: targetassets.IgnitionConfigs,
9185
}
9286

9387
clusterTarget = target{
@@ -128,7 +122,7 @@ var (
128122
}
129123
},
130124
},
131-
assets: []asset.WritableAsset{&cluster.TerraformVariables{}, &kubeconfig.Admin{}, &tls.JournalCertKey{}, &cluster.Metadata{}, &cluster.Cluster{}},
125+
assets: targetassets.Cluster,
132126
}
133127

134128
targets = []target{installConfigTarget, manifestTemplatesTarget, manifestsTarget, ignitionConfigsTarget, clusterTarget}
@@ -154,7 +148,7 @@ func newCreateCmd() *cobra.Command {
154148

155149
func runTargetCmd(targets ...asset.WritableAsset) func(cmd *cobra.Command, args []string) {
156150
runner := func(directory string) error {
157-
assetStore, err := asset.NewStore(directory)
151+
assetStore, err := assetstore.NewStore(directory)
158152
if err != nil {
159153
return errors.Wrapf(err, "failed to create asset store")
160154
}

‎cmd/openshift-install/destroy.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import (
55
"github.com/sirupsen/logrus"
66
"github.com/spf13/cobra"
77

8-
"github.com/openshift/installer/pkg/asset"
8+
assetstore "github.com/openshift/installer/pkg/asset/store"
99
"github.com/openshift/installer/pkg/destroy"
1010
"github.com/openshift/installer/pkg/destroy/bootstrap"
1111
_ "github.com/openshift/installer/pkg/destroy/libvirt"
@@ -52,7 +52,7 @@ func runDestroyCmd(directory string) error {
5252
return errors.Wrap(err, "Failed to destroy cluster")
5353
}
5454

55-
store, err := asset.NewStore(directory)
55+
store, err := assetstore.NewStore(directory)
5656
if err != nil {
5757
return errors.Wrapf(err, "failed to create asset store")
5858
}

‎docs/design/resource_dep.svg

Lines changed: 728 additions & 578 deletions
Loading

‎pkg/asset/asset.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"io/ioutil"
66
"os"
77
"path/filepath"
8+
"sort"
89

910
"github.com/pkg/errors"
1011
"github.com/sirupsen/logrus"
@@ -58,9 +59,9 @@ func PersistToFile(asset WritableAsset, directory string) error {
5859
return nil
5960
}
6061

61-
// deleteAssetFromDisk removes all the files for asset from disk.
62+
// DeleteAssetFromDisk removes all the files for asset from disk.
6263
// this is function is not safe for calling concurrently on the same directory.
63-
func deleteAssetFromDisk(asset WritableAsset, directory string) error {
64+
func DeleteAssetFromDisk(asset WritableAsset, directory string) error {
6465
logrus.Debugf("Purging asset %q from disk", asset.Name())
6566
for _, f := range asset.Files() {
6667
path := filepath.Join(directory, f.Filename)
@@ -95,3 +96,8 @@ func isDirEmpty(name string) (bool, error) {
9596
}
9697
return false, err // Either not empty or error, suits both cases
9798
}
99+
100+
// SortFiles sorts the specified files by file name.
101+
func SortFiles(files []*File) {
102+
sort.Slice(files, func(i, j int) bool { return files[i].Filename < files[j].Filename })
103+
}

‎pkg/asset/filefetcher.go

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,5 @@
11
package asset
22

3-
import (
4-
"io/ioutil"
5-
"path/filepath"
6-
"sort"
7-
)
8-
93
//go:generate mockgen -source=./filefetcher.go -destination=./mock/filefetcher_generated.go -package=mock
104

115
// FileFetcher fetches the asset files from disk.
@@ -15,45 +9,3 @@ type FileFetcher interface {
159
// FetchByPattern returns the files whose name match the given glob.
1610
FetchByPattern(pattern string) ([]*File, error)
1711
}
18-
19-
type fileFetcher struct {
20-
directory string
21-
}
22-
23-
// FetchByName returns the file with the given name.
24-
func (f *fileFetcher) FetchByName(name string) (*File, error) {
25-
data, err := ioutil.ReadFile(filepath.Join(f.directory, name))
26-
if err != nil {
27-
return nil, err
28-
}
29-
return &File{Filename: name, Data: data}, nil
30-
}
31-
32-
// FetchByPattern returns the files whose name match the given regexp.
33-
func (f *fileFetcher) FetchByPattern(pattern string) (files []*File, err error) {
34-
matches, err := filepath.Glob(filepath.Join(f.directory, pattern))
35-
if err != nil {
36-
return nil, err
37-
}
38-
39-
files = make([]*File, 0, len(matches))
40-
for _, path := range matches {
41-
data, err := ioutil.ReadFile(path)
42-
if err != nil {
43-
return nil, err
44-
}
45-
46-
filename, err := filepath.Rel(f.directory, path)
47-
if err != nil {
48-
return nil, err
49-
}
50-
51-
files = append(files, &File{
52-
Filename: filename,
53-
Data: data,
54-
})
55-
}
56-
57-
sort.Slice(files, func(i, j int) bool { return files[i].Filename < files[j].Filename })
58-
return files, nil
59-
}

‎pkg/asset/machines/master.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,10 @@ func (m *Master) Generate(dependencies asset.Parents) error {
111111
}
112112
m.MachinesRaw = raw
113113
case nonetypes.Name:
114+
// This is needed to ensure that roundtrip generate-load tests pass when
115+
// comparing this value. Otherwise, generate will use a nil value while
116+
// load will use an empty byte slice.
117+
m.MachinesRaw = []byte{}
114118
case openstacktypes.Name:
115119
mpool := defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName)
116120
mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform)

‎pkg/asset/machines/worker.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ func (w *Worker) Generate(dependencies asset.Parents) error {
131131
}
132132
w.MachineSetRaw = raw
133133
case nonetypes.Name:
134+
// This is needed to ensure that roundtrip generate-load tests pass when
135+
// comparing this value. Otherwise, generate will use a nil value while
136+
// load will use an empty byte slice.
137+
w.MachineSetRaw = []byte{}
134138
case openstacktypes.Name:
135139
mpool := defaultOpenStackMachinePoolPlatform(ic.Platform.OpenStack.FlavorName)
136140
mpool.Set(ic.Platform.OpenStack.DefaultMachinePlatform)

‎pkg/asset/manifests/dns.go

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package manifests
22

33
import (
4-
"os"
54
"path/filepath"
65

76
"github.com/ghodss/yaml"
@@ -22,7 +21,6 @@ var (
2221

2322
// DNS generates the cluster-dns-*.yml files.
2423
type DNS struct {
25-
config *configv1.DNS
2624
FileList []*asset.File
2725
}
2826

@@ -46,7 +44,7 @@ func (d *DNS) Generate(dependencies asset.Parents) error {
4644
installConfig := &installconfig.InstallConfig{}
4745
dependencies.Get(installConfig)
4846

49-
d.config = &configv1.DNS{
47+
config := &configv1.DNS{
5048
TypeMeta: metav1.TypeMeta{
5149
APIVersion: configv1.SchemeGroupVersion.String(),
5250
Kind: "DNS",
@@ -60,7 +58,7 @@ func (d *DNS) Generate(dependencies asset.Parents) error {
6058
},
6159
}
6260

63-
configData, err := yaml.Marshal(d.config)
61+
configData, err := yaml.Marshal(config)
6462
if err != nil {
6563
return errors.Wrapf(err, "failed to create %s manifests from InstallConfig", d.Name())
6664
}
@@ -91,31 +89,5 @@ func (d *DNS) Files() []*asset.File {
9189

9290
// Load loads the already-rendered files back from disk.
9391
func (d *DNS) Load(f asset.FileFetcher) (bool, error) {
94-
crdFile, err := f.FetchByName(filepath.Join(manifestDir, dnsCrdFilename))
95-
if err != nil {
96-
if os.IsNotExist(err) {
97-
return false, nil
98-
}
99-
return false, err
100-
}
101-
102-
cfgFile, err := f.FetchByName(dnsCfgFilename)
103-
if err != nil {
104-
if os.IsNotExist(err) {
105-
return false, nil
106-
}
107-
108-
return false, err
109-
}
110-
111-
dnsConfig := &configv1.DNS{}
112-
if err := yaml.Unmarshal(cfgFile.Data, dnsConfig); err != nil {
113-
return false, errors.Wrapf(err, "failed to unmarshal %s", dnsCfgFilename)
114-
}
115-
116-
fileList := []*asset.File{crdFile, cfgFile}
117-
118-
d.FileList, d.config = fileList, dnsConfig
119-
120-
return true, nil
92+
return false, nil
12193
}

‎pkg/asset/manifests/ingress.go

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package manifests
22

33
import (
44
"fmt"
5-
"os"
65
"path/filepath"
76

87
"github.com/ghodss/yaml"
@@ -23,7 +22,6 @@ var (
2322

2423
// Ingress generates the cluster-ingress-*.yml files.
2524
type Ingress struct {
26-
config *configv1.Ingress
2725
FileList []*asset.File
2826
}
2927

@@ -47,7 +45,7 @@ func (ing *Ingress) Generate(dependencies asset.Parents) error {
4745
installConfig := &installconfig.InstallConfig{}
4846
dependencies.Get(installConfig)
4947

50-
ing.config = &configv1.Ingress{
48+
config := &configv1.Ingress{
5149
TypeMeta: metav1.TypeMeta{
5250
APIVersion: configv1.SchemeGroupVersion.String(),
5351
Kind: "Ingress",
@@ -61,7 +59,7 @@ func (ing *Ingress) Generate(dependencies asset.Parents) error {
6159
},
6260
}
6361

64-
configData, err := yaml.Marshal(ing.config)
62+
configData, err := yaml.Marshal(config)
6563
if err != nil {
6664
return errors.Wrapf(err, "failed to create %s manifests from InstallConfig", ing.Name())
6765
}
@@ -90,33 +88,7 @@ func (ing *Ingress) Files() []*asset.File {
9088
return ing.FileList
9189
}
9290

93-
// Load loads the already-rendered files back from disk.
91+
// Load returns false since this asset is not written to disk by the installer.
9492
func (ing *Ingress) Load(f asset.FileFetcher) (bool, error) {
95-
crdFile, err := f.FetchByName(filepath.Join(manifestDir, ingCrdFilename))
96-
if err != nil {
97-
if os.IsNotExist(err) {
98-
return false, nil
99-
}
100-
return false, err
101-
}
102-
103-
cfgFile, err := f.FetchByName(ingCfgFilename)
104-
if err != nil {
105-
if os.IsNotExist(err) {
106-
return false, nil
107-
}
108-
109-
return false, err
110-
}
111-
112-
ingressConfig := &configv1.Ingress{}
113-
if err := yaml.Unmarshal(cfgFile.Data, ingressConfig); err != nil {
114-
return false, errors.Wrapf(err, "failed to unmarshal %s", ingCfgFilename)
115-
}
116-
117-
fileList := []*asset.File{crdFile, cfgFile}
118-
119-
ing.FileList, ing.config = fileList, ingressConfig
120-
121-
return true, nil
93+
return false, nil
12294
}

0 commit comments

Comments
 (0)