Skip to content

for review of vsphere advanced capabilities - #56

Closed
DaanHoogland wants to merge 149 commits into
masterfrom
vsphereAdvancedCapabilities
Closed

for review of vsphere advanced capabilities#56
DaanHoogland wants to merge 149 commits into
masterfrom
vsphereAdvancedCapabilities

Conversation

@DaanHoogland

Copy link
Copy Markdown
Member

Description

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Screenshots (if appropriate):

How Has This Been Tested?

@DaanHoogland
DaanHooglandforce-pushed the vsphereAdvancedCapabilities branch from 50e8709 to de099c0CompareJune 12, 2020 10:46
return true;
}

//TODO: Remove this method after deploying VM from OVF directly using content library

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

👍

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

on second thought, we need to be backwards compatible. I don't think we can do this with a single transition script.

@sureshanaparti
sureshanapartiforce-pushed the vsphereAdvancedCapabilities branch from 14b90b0 to 15cdd27CompareJuly 2, 2020 14:18

@DaanHooglandDaanHoogland left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

thanks Suresh, i'll start a build to deploy and test tomorrow

createSpec.setDescription("Local content library for datastore " + datastoreName);
createSpec.setType(LibraryModel.LibraryType.LOCAL);
createSpec.setStorageBackings(Collections.singletonList(storageBacking));

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

sounds like a method should be create called buildTheSpecificationForTheLibraryToBeCreated(...)

Comment on lines +97 to +139
//Get the storage backing on the datastore
StorageBacking dsStorageBacking = createStorageBacking(context, datastoreName);
boolean canDelete = false;
for (Iterator<StorageBacking> iterator = localLibrary.getStorageBackings().iterator(); iterator.hasNext();) {
StorageBacking storageBacking = (StorageBacking) iterator.next();
if(dsStorageBacking.equals(storageBacking)) {
canDelete = true;
break;
}
}

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

getTheStorageBackingOnTheDatastore(...)

@DaanHoogland

Copy link
Copy Markdown
MemberAuthor

@sureshanaparti please don't force push on a shared branch. We can clean up before a public PR.

@sureshanaparti
sureshanapartiforce-pushed the vsphereAdvancedCapabilities branch from eef8701 to a268d94CompareJuly 2, 2020 20:18
authorized = {RoleType.Admin})
public class ImportVsphereStoragePoliciesCmd extends BaseCmd {

public static final Logger s_logger = Logger.getLogger(ImportVsphereStoragePoliciesCmd.class.getName());

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@harikrishna-patnala , can you make this static final Logger LOGGER?


/**
* deploys a new VM from a ovf spec. It ignores network, defaults locale to 'US'
* @param host

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@DaanHoogland (note to self) remove un-annotated annotation tags


public class PbmPlacementSolverMO extends BaseMO {

private static final Logger s_logger = Logger.getLogger(PbmPlacementSolverMO.class);

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

statis final naming


public class PbmProfileManagerMO extends BaseMO {

private static final Logger s_logger = Logger.getLogger(PbmProfileManagerMO.class);

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

statis final naming


public class VirtualStorageObjectManagerMO extends BaseMO {
@SuppressWarnings("unused")
private static final Logger s_logger = Logger.getLogger(VirtualStorageObjectManagerMO.class);

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

statis final naming

import com.vmware.vcenter.ovf.LibraryItem;

public class ContentLibraryClient implements VmwareClientService {
private static final Logger s_logger = Logger.getLogger(ContentLibraryClient.class);

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

statis final naming

@yadvr

yadvr commented Jul 8, 2020

Copy link
Copy Markdown

@harikrishna-patnala@DaanHoogland can you merge latest master and address the conflicts. I'll start some test/review work in 2 hrs. Thnx.

@Pearl1594
Pearl1594force-pushed the vsphereAdvancedCapabilities branch from 62c82d8 to d5453baCompareJuly 28, 2020 08:17
Comment on lines +215 to +218
INSERT IGNORE INTO `cloud`.`disk_offering` (name, uuid, display_text, customized, unique_name, disk_size, system_use, type, display_offering)
VALUES ('Custom Deploy-as-is Instance', UUID(), 'Custom Deploy-as-is Instance', 1, 'ApacheCloudStack.org-Custom Deploy-as-is Instance', 0, 0, 'Service', 0);
INSERT IGNORE INTO `cloud`.`service_offering` (`id`, `nw_rate`, `mc_rate`) VALUES
((SELECT `id` FROM `cloud`.`disk_offering` WHERE `unique_name` = 'Cloud.com-Custom Deploy-as-is Instance'), null, null);
((SELECT `id` FROM `cloud`.`disk_offering` WHERE `unique_name` = 'ApacheCloudStack.org-Custom Deploy-as-is Instance'), null, null);

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@shwstppr this change will let you select the custom offering to use as a quick fix.

  • UUID as generated in the diskoffering
  • uniquename = ApacheCloudStack.org-Custom Deploy-as-is Instance

Comment on lines +740 to +743
Answer answer = new Answer(null);
CopyCommandResult result = new CopyCommandResult(null, null);
result.setSuccess(true);
caller.complete(result);

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I must say this is a terrible hack, maybe we should rename the method to reflect that the copy may not even be attempted?

shwstppr pushed a commit that referenced this pull request Sep 22, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@DaanHoogland@yadvr@nvazquez@harikrishna-patnala@sureshanaparti