Skip to content

Commit

Permalink
wait-for install-complete: Fix re-generation of install config
Browse files Browse the repository at this point in the history
If the cluster is created and the wait-for install-complete command
is called to check the status, the installer checks if the install
config exists to get the platform information to set the timeout for
baremetal clusters a little higher. Since the install config is
consumed during the cluster creation, the installer will then start
to re-generate the install config and ask the user to provide the
information again.

Modified the platform information gathering to only pick the information
up only if the information is available and if it does not exist, set the
timeout value to the maximum value required as default. This will
now avoid re-generation of the install config.
  • Loading branch information
rna-afk committed May 28, 2020
1 parent 7f3456f commit 8846ff7
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 5 deletions.
9 changes: 4 additions & 5 deletions cmd/openshift-install/create.go
Expand Up @@ -343,14 +343,13 @@ func waitForBootstrapConfigMap(ctx context.Context, client *kubernetes.Clientset
// waitForInitializedCluster watches the ClusterVersion waiting for confirmation
// that the cluster has been initialized.
func waitForInitializedCluster(ctx context.Context, config *rest.Config) error {
timeout := 30 * time.Minute
timeout := 60 * time.Minute

// Wait longer for baremetal, due to length of time it takes to boot
if assetStore, err := assetstore.NewStore(rootOpts.dir); err == nil {
installConfig := &installconfig.InstallConfig{}
if err := assetStore.Fetch(installConfig); err == nil {
if installConfig.Config.Platform.Name() == baremetal.Name {
timeout = 60 * time.Minute
if installConfig, err := assetStore.Load(&installconfig.InstallConfig{}); err == nil && installConfig != nil {
if installConfig.(*installconfig.InstallConfig).Config.Platform.Name() != baremetal.Name {
timeout = 30 * time.Minute
}
}
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/asset/store.go
Expand Up @@ -14,4 +14,8 @@ type Store interface {
// DestroyState removes everything from the internal state and the internal
// state file
DestroyState() error

// Load retrieves the state of the given asset but does not generate it if it
// does not exist and instead will return nil if not found.
Load(Asset) (Asset, error)
}
15 changes: 15 additions & 0 deletions pkg/asset/store/store.go
Expand Up @@ -357,3 +357,18 @@ func (s *storeImpl) purge(excluded []asset.WritableAsset) error {
func increaseIndent(indent string) string {
return indent + " "
}

// Load retrieves the given asset if it is present in the store and does not generate the asset
// if it does not exist and will return nil.
func (s *storeImpl) Load(a asset.Asset) (asset.Asset, error) {
foundOnDisk, err := s.load(a, "")
if err != nil {
return nil, errors.Wrap(err, "failed to load asset")
}

if foundOnDisk.source == unfetched {
return nil, nil
}

return s.assets[reflect.TypeOf(a)].asset, nil
}
55 changes: 55 additions & 0 deletions pkg/asset/store/store_test.go
Expand Up @@ -429,3 +429,58 @@ func TestStoreFetchIdempotency(t *testing.T) {
filepath.Walk(tempDir, walkFunc)
assert.Equal(t, expectedFiles, actualFiles, "unexpected files on disk")
}

func TestStoreLoadOnDiskAssets(t *testing.T) {
cases := []struct {
name string
assets map[string][]string
onDiskAssets []string
target string
expectedFoundValue bool
}{
{
name: "on-disk assets",
assets: map[string][]string{
"a": {},
},
onDiskAssets: []string{"a"},
target: "a",
expectedFoundValue: true,
},
{
name: "no on-disk assets",
assets: map[string][]string{
"a": {"b"},
"b": {},
},
onDiskAssets: nil,
target: "a",
expectedFoundValue: false,
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
clearAssetBehaviors()
store := &storeImpl{
assets: map[reflect.Type]*assetState{},
}
assets := make(map[string]asset.Asset, len(tc.assets))
for name := range tc.assets {
assets[name] = newTestStoreAsset(name)
}
for name, deps := range tc.assets {
dependenciesOfAsset := make([]asset.Asset, len(deps))
for i, d := range deps {
dependenciesOfAsset[i] = assets[d]
}
dependencies[reflect.TypeOf(assets[name])] = dependenciesOfAsset
}
for _, name := range tc.onDiskAssets {
onDiskAssets[reflect.TypeOf(assets[name])] = true
}
found, err := store.Load(assets[tc.target])
assert.NoError(t, err, "unexpected error")
assert.EqualValues(t, tc.expectedFoundValue, found != nil)
})
}
}

0 comments on commit 8846ff7

Please sign in to comment.