Skip to content

Conversation

@jcpowermac
Copy link
Contributor

  • Modify clustermetadata to include VSphere
  • Add additional fields for auth to vsphere Metadata struct
    that is required for accessing vSphere vCenter
    to destroy cluster infrastructure objects
  • vSphere Destroy
    • Create clients for both soap and rest
    • Finds the vCenter objects attached to the Tag
    • Retieve the ManagedObjects for both Folder and VirtualMachine
    • Confirm that exactly one Folder is available
    • Remove VirtualMachines that are a child of Folder
    • Remove Folder if empty

@openshift-ci-robot openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 8, 2020
@jcpowermac
Copy link
Contributor Author

Need to update Gopkg (will do in another commit)

@jcpowermac
Copy link
Contributor Author

@patrickdillon
@mtnbikenc
when you have a moment if I can get an initial review.

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jan 8, 2020
@jcpowermac
Copy link
Contributor Author

Looks like:
https://github.com/openshift/machine-api-operator/blob/master/go.mod#L28
is using govmomi version v0.21.0 we might want to use that version as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jcpowermac jcpowermac changed the title [wip] Add vSphere IPI destroy Add vSphere IPI destroy Jan 16, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 16, 2020
@jcpowermac
Copy link
Contributor Author

/retest

Copy link
Contributor

@patrickdillon patrickdillon left a comment

Choose a reason for hiding this comment

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

Currently there is no output when you run destroy at the standard log level. I left a couple of suggestions of where I definitely think we should switch debug to info but I think that is a minimum and we could be more verbose.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: regroup these internal imports to:
stdlib

external

internal

Copy link
Contributor

Choose a reason for hiding this comment

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

This output should be shown at the default log-level so virtualMachineLogger.Debug -> virtualMachineLogger.Info

Copy link
Contributor

Choose a reason for hiding this comment

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

.Debug -> .Info

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment can be removed (or updated as we are using InfraID)

- Modify clustermetadata to include VSphere
- Add additional fields for auth to vsphere Metadata struct
  that is required for accessing vSphere vCenter
  to destroy cluster infrastructure objects
- vSphere Destroy
  - Create clients for both soap and rest
  - Finds the vCenter objects attached to the Tag
  - Retieve the ManagedObjects for both Folder and VirtualMachine
  - Confirm that exactly one Folder is available
  - Remove VirtualMachines that are a child of Folder
  - Remove Folder if empty

vsphere missing metadata

- Modified `pkg/asset/cluster/metadata.go`
  to include the vsphere case for creating the metadata asset
- Created vsphere metadata function to populate metadata struct
  with auth from install config.

Create vsphere `client.go` that contains a single function
to create vSphere SOAP and REST clients that will be
used for at least destroy and gather bootstrap
@patrickdillon
Copy link
Contributor

Logging changes look good to me.
Before change there was no output:

$ ./openshift-install create cluster --dir vsphere
$

Now deleted resources are output:

$ ./openshift-install destroy cluster --dir vsphere
INFO Destroyed                                     VirtualMachine=test-cluster-v2mww-master-1
INFO Destroyed                                     VirtualMachine=test-cluster-v2mww-master-0
INFO Destroyed                                     VirtualMachine=test-cluster-v2mww-master-2
INFO Destroyed                                     VirtualMachine=test-cluster-v2mww-bootstrap
INFO Destroyed                                     Folder=test-cluster-v2mww
$

For reference, here is log with --log-level debug

$ ./openshift-install destroy cluster --dir vsphere --log-level debug
DEBUG OpenShift Installer unreleased-master-2383-gaade1eb75561332885c102eac3ebd2af3c6e8b20 
DEBUG Built from commit aade1eb75561332885c102eac3ebd2af3c6e8b20 
DEBUG find attached objects on tag                 
DEBUG find Folder objects                          
DEBUG find VirtualMachine objects                  
DEBUG delete VirtualMachines                       
DEBUG Powered off                                   VirtualMachine=test-cluster-sp8d9-master-0
INFO Destroyed                                     VirtualMachine=test-cluster-sp8d9-master-0
DEBUG Powered off                                   VirtualMachine=test-cluster-sp8d9-master-2
INFO Destroyed                                     VirtualMachine=test-cluster-sp8d9-master-2
DEBUG Powered off                                   VirtualMachine=test-cluster-sp8d9-master-1
INFO Destroyed                                     VirtualMachine=test-cluster-sp8d9-master-1
DEBUG Powered off                                   VirtualMachine=test-cluster-sp8d9-bootstrap
INFO Destroyed                                     VirtualMachine=test-cluster-sp8d9-bootstrap
DEBUG find Folder objects                          
DEBUG delete Folder                                
INFO Destroyed                                     Folder=test-cluster-sp8d9
DEBUG Purging asset "Terraform Variables" from disk 
DEBUG Purging asset "Kubeconfig Admin Client" from disk 
DEBUG Purging asset "Kubeadmin Password" from disk 
DEBUG Purging asset "Certificate (journal-gatewayd)" from disk 
DEBUG Purging asset "Metadata" from disk           
DEBUG Purging asset "Cluster" from disk            
$ 

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 20, 2020
@jcpowermac
Copy link
Contributor Author

/assign @sdodson

@sdodson
Copy link
Member

sdodson commented Jan 20, 2020

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 20, 2020
@sdodson
Copy link
Member

sdodson commented Jan 20, 2020

/approve

@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sdodson

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:

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

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@jcpowermac
Copy link
Contributor Author

/hold
e2e-aws-upgrade broken

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 21, 2020
@jstuever jstuever mentioned this pull request Jan 21, 2020
3 tasks
@jcpowermac
Copy link
Contributor Author

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 22, 2020
@jcpowermac
Copy link
Contributor Author

/retest

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

21 similar comments
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit b9d46c7 into openshift:master Jan 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants