Skip to content

Delete old VCH#3116

Merged
emlin merged 4 commits intovmware:masterfrom
emlin:2719
Nov 15, 2016
Merged

Delete old VCH#3116
emlin merged 4 commits intovmware:masterfrom
emlin:2719

Conversation

@emlin
Copy link
Contributor

@emlin emlin commented Nov 11, 2016

Fixes #2719
If vic-machine identified the vm is one VCH instance, will delete the container VMs and VCH instance itself. Even the guestinfo configuration is changed, and some of configuration cannot be identified.
The problem is that if there is custom image store, or volume store configured, but vic-machine cannot read those configuration back, those resources will not be cleaned up.

return errors.New("delete failed")
}

log.Warnf("VCH version %q is different with installer version %s. Force delete will try best to delete everything found.", vchConfig.Version.ShortVersion(), installerBuild.ShortVersion())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change warning text to: Force delete will attempt to remove everything related to the installed VCH.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@emlin emlin changed the title [full ci]Delete old VCH Delete old VCH Nov 11, 2016
installerBuild := version.GetBuild()
if vchConfig.Version == nil || !installerBuild.Equal(vchConfig.Version) {
if !d.Data.Force {
log.Errorf("VCH version %q is different with installer version %s. Specify --force to force delete", vchConfig.Version.ShortVersion(), installerBuild.ShortVersion())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

different than installer version

return errors.New("delete failed")
}

log.Warnf("VCH version %q is different with installer version %s. Force delete will attempt to remove everything related to the installed VCH", vchConfig.Version.ShortVersion(), installerBuild.ShortVersion())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

err = errors.Errorf("Cannot find compute resources from configuration")
return nil, err
}
log.Warnf("Cannot find compute resources from configuration, try to delete under parent resource pool")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attempting to delete

err = errors.Errorf("Cannot find image stores from configuration")
return nil, err
}
log.Warnf("Cannot find image stores from configuration, try to delete from vm datastore")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@andrewtchin
Copy link
Contributor

lgtm

rp := compute.NewResourcePool(d.ctx, d.session, rpRef)

if d.parentResourcepool == nil {
log.Warnf("Do not find parent VCH resource pool")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor - "Did not find..."

Log ${output}
Should Be Equal As Integers ${rc} 0
Should Not Contain ${output} Error
${name}= Generate Random String 15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL 👍

@anchal-agrawal
Copy link
Contributor

LGTM

@emlin
Copy link
Contributor Author

emlin commented Nov 15, 2016

.

@emlin emlin merged commit f83f173 into vmware:master Nov 15, 2016
fdawg4l pushed a commit to fdawg4l/vic that referenced this pull request Nov 16, 2016
* Try to delete old vch

* add vic-machine backward compatibility test

* update message

* comments
@emlin emlin deleted the 2719 branch May 26, 2017 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants