weizhouapache commented on code in PR #12357:
URL: https://github.com/apache/cloudstack/pull/12357#discussion_r2792330705
##########
api/src/main/java/com/cloud/event/EventTypes.java:
##########
@@ -630,6 +631,7 @@ public class EventTypes {
// Backup and Recovery events
public static final String EVENT_VM_BACKUP_IMPORT_OFFERING =
"BACKUP.IMPORT.OFFERING";
+ public static final String EVENT_VM_BACKUP_CLONE_OFFERING =
"BACKUP.CLONE.OFFERING";
Review Comment:
~~event types for clone of other offerings (network, disk, vpc) ?~~
updated: I see, some APIs are sync.
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/backup/CloneBackupOfferingCmd.java:
##########
@@ -0,0 +1,167 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.backup;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.offering.DomainAndZoneIdResolver;
+import org.apache.cloudstack.api.response.BackupOfferingResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.backup.BackupManager;
+import org.apache.cloudstack.backup.BackupOffering;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.LongFunction;
+
+@APICommand(name = "cloneBackupOffering",
+ description = "Clones a backup offering from an existing offering",
+ responseObject = BackupOfferingResponse.class, since = "4.23.0",
+ authorized = {RoleType.Admin})
+public class CloneBackupOfferingCmd extends BaseAsyncCmd implements
DomainAndZoneIdResolver {
+
+ @Inject
+ protected BackupManager backupManager;
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ ////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.SOURCE_OFFERING_ID, type =
BaseCmd.CommandType.UUID, entityType = BackupOfferingResponse.class,
+ required = true, description = "The ID of the source backup
offering to clone from")
+ private Long sourceOfferingId;
+
+ @Parameter(name = ApiConstants.NAME, type = BaseCmd.CommandType.STRING,
required = true,
+ description = "The name of the cloned offering")
+ private String name;
+
+ @Parameter(name = ApiConstants.DESCRIPTION, type =
BaseCmd.CommandType.STRING, required = false,
+ description = "The description of the cloned offering")
+ private String description;
+
+ @Parameter(name = ApiConstants.EXTERNAL_ID, type =
BaseCmd.CommandType.STRING, required = false,
+ description = "The backup offering ID (from backup provider side)")
+ private String externalId;
+
+ @Parameter(name = ApiConstants.ZONE_ID, type = BaseCmd.CommandType.UUID,
entityType = ZoneResponse.class,
+ description = "The zone ID", required = false)
+ private Long zoneId;
+
+ @Parameter(name = ApiConstants.DOMAIN_ID,
+ type = CommandType.STRING,
+ description = "the ID of the containing domain(s) as comma
separated string, public for public offerings",
+ since = "4.23.0",
Review Comment:
the API already has a `since = "4.23.0"`
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/backup/ImportBackupOfferingCmd.java:
##########
@@ -86,7 +86,8 @@ public class ImportBackupOfferingCmd extends BaseAsyncCmd {
type = CommandType.LIST,
collectionType = CommandType.UUID,
entityType = DomainResponse.class,
- description = "the ID of the containing domain(s), null for public
offerings")
+ description = "the ID of the containing domain(s), null for public
offerings",
+ since = "4.23.0")
Review Comment:
is `since` needed here ?
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmd.java:
##########
@@ -0,0 +1,113 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.network;
+
+import java.util.List;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.NetworkOfferingResponse;
+
+import com.cloud.offering.NetworkOffering;
+
+@APICommand(name = "cloneNetworkOffering",
+ description = "Clones a network offering. All parameters are copied
from the source offering unless explicitly overridden. " +
+ "Use 'addServices' and 'dropServices' to modify the service
list without respecifying everything.",
+ responseObject = NetworkOfferingResponse.class,
+ requestHasSensitiveInfo = false,
+ responseHasSensitiveInfo = false,
+ since = "4.23.0")
Review Comment:
Admin only ?
##########
api/src/main/java/com/cloud/event/EventTypes.java:
##########
@@ -630,6 +631,7 @@ public class EventTypes {
// Backup and Recovery events
public static final String EVENT_VM_BACKUP_IMPORT_OFFERING =
"BACKUP.IMPORT.OFFERING";
+ public static final String EVENT_VM_BACKUP_CLONE_OFFERING =
"BACKUP.CLONE.OFFERING";
Review Comment:
the values may be better consistent:
XXXX.OFFERING.CLONE
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/vpc/CreateVPCOfferingCmd.java:
##########
@@ -179,9 +178,7 @@ public boolean isExternalNetworkProvider() {
}
public List<String> getSupportedServices() {
- if (!isExternalNetworkProvider() &&
CollectionUtils.isEmpty(supportedServices)) {
Review Comment:
~~is this check moved to another place ?~~
updated: I see, moved to
`server/src/main/java/com/cloud/network/vpc/VpcManagerImpl.java`
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -325,6 +326,79 @@ public BackupOffering importBackupOffering(final
ImportBackupOfferingCmd cmd) {
return savedOffering;
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_VM_BACKUP_CLONE_OFFERING,
eventDescription = "cloning backup offering", create = true)
+ public BackupOffering cloneBackupOffering(final CloneBackupOfferingCmd
cmd) {
+ final BackupOfferingVO sourceOffering =
backupOfferingDao.findById(cmd.getSourceOfferingId());
+ if (sourceOffering == null) {
+ throw new InvalidParameterValueException("Unable to find backup
offering with ID: " + cmd.getSourceOfferingId());
+ }
+
+ validateBackupForZone(sourceOffering.getZoneId());
+
+ if (backupOfferingDao.findByName(cmd.getName(),
sourceOffering.getZoneId()) != null) {
+ throw new CloudRuntimeException("A backup offering with the name
'" + cmd.getName() + "' already exists in this zone");
+ }
+
+ final String description = cmd.getDescription() != null ?
cmd.getDescription() : sourceOffering.getDescription();
+ final String externalId = cmd.getExternalId() != null ?
cmd.getExternalId() : sourceOffering.getExternalId();
+ final boolean userDrivenBackups = cmd.getUserDrivenBackups() != null ?
cmd.getUserDrivenBackups() : sourceOffering.isUserDrivenBackupAllowed();
+
+ if (!externalId.equals(sourceOffering.getExternalId())) {
+ final BackupProvider provider =
getBackupProvider(sourceOffering.getZoneId());
+ if (!provider.isValidProviderOffering(sourceOffering.getZoneId(),
externalId)) {
+ throw new CloudRuntimeException("Backup offering '" +
externalId + "' does not exist on provider " + provider.getName() + " on zone "
+ sourceOffering.getZoneId());
+ }
+ }
+
+ if (!externalId.equals(sourceOffering.getExternalId())) {
Review Comment:
is this condition check correct ?
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmd.java:
##########
@@ -0,0 +1,113 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.network;
+
+import java.util.List;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.NetworkOfferingResponse;
+
+import com.cloud.offering.NetworkOffering;
+
+@APICommand(name = "cloneNetworkOffering",
+ description = "Clones a network offering. All parameters are copied
from the source offering unless explicitly overridden. " +
+ "Use 'addServices' and 'dropServices' to modify the service
list without respecifying everything.",
+ responseObject = NetworkOfferingResponse.class,
+ requestHasSensitiveInfo = false,
+ responseHasSensitiveInfo = false,
+ since = "4.23.0")
+public class CloneNetworkOfferingCmd extends NetworkOfferingBaseCmd {
Review Comment:
oh, now I know why there is not event type for this API: it is sync
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/backup/CloneBackupOfferingCmd.java:
##########
@@ -0,0 +1,167 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.backup;
+
+import javax.inject.Inject;
+
+import org.apache.cloudstack.acl.RoleType;
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseAsyncCmd;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.command.offering.DomainAndZoneIdResolver;
+import org.apache.cloudstack.api.response.BackupOfferingResponse;
+import org.apache.cloudstack.api.response.ZoneResponse;
+import org.apache.cloudstack.backup.BackupManager;
+import org.apache.cloudstack.backup.BackupOffering;
+import org.apache.cloudstack.context.CallContext;
+
+import com.cloud.event.EventTypes;
+import com.cloud.exception.ConcurrentOperationException;
+import com.cloud.exception.InsufficientCapacityException;
+import com.cloud.exception.InvalidParameterValueException;
+import com.cloud.exception.NetworkRuleConflictException;
+import com.cloud.exception.ResourceAllocationException;
+import com.cloud.exception.ResourceUnavailableException;
+import com.cloud.utils.exception.CloudRuntimeException;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.function.LongFunction;
+
+@APICommand(name = "cloneBackupOffering",
+ description = "Clones a backup offering from an existing offering",
+ responseObject = BackupOfferingResponse.class, since = "4.23.0",
+ authorized = {RoleType.Admin})
+public class CloneBackupOfferingCmd extends BaseAsyncCmd implements
DomainAndZoneIdResolver {
+
+ @Inject
+ protected BackupManager backupManager;
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ ////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.SOURCE_OFFERING_ID, type =
BaseCmd.CommandType.UUID, entityType = BackupOfferingResponse.class,
+ required = true, description = "The ID of the source backup
offering to clone from")
+ private Long sourceOfferingId;
+
+ @Parameter(name = ApiConstants.NAME, type = BaseCmd.CommandType.STRING,
required = true,
+ description = "The name of the cloned offering")
+ private String name;
+
+ @Parameter(name = ApiConstants.DESCRIPTION, type =
BaseCmd.CommandType.STRING, required = false,
+ description = "The description of the cloned offering")
+ private String description;
+
+ @Parameter(name = ApiConstants.EXTERNAL_ID, type =
BaseCmd.CommandType.STRING, required = false,
+ description = "The backup offering ID (from backup provider side)")
+ private String externalId;
+
+ @Parameter(name = ApiConstants.ZONE_ID, type = BaseCmd.CommandType.UUID,
entityType = ZoneResponse.class,
+ description = "The zone ID", required = false)
+ private Long zoneId;
+
+ @Parameter(name = ApiConstants.DOMAIN_ID,
+ type = CommandType.STRING,
+ description = "the ID of the containing domain(s) as comma
separated string, public for public offerings",
+ since = "4.23.0",
+ length = 4096)
+ private String domainIds;
+
+ @Parameter(name = ApiConstants.ALLOW_USER_DRIVEN_BACKUPS, type =
BaseCmd.CommandType.BOOLEAN,
+ description = "Whether users are allowed to create adhoc backups
and backup schedules", required = false)
+ private Boolean userDrivenBackups;
Review Comment:
some of the API parameters are used in CreateBackupOfferingCmd.
will you create a BackupOfferingBase, similar to NetworkOfferingBase you
have created ?
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/network/CloneNetworkOfferingCmd.java:
##########
@@ -0,0 +1,113 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+package org.apache.cloudstack.api.command.admin.network;
+
+import java.util.List;
+
+import org.apache.cloudstack.api.APICommand;
+import org.apache.cloudstack.api.ApiConstants;
+import org.apache.cloudstack.api.ApiErrorCode;
+import org.apache.cloudstack.api.BaseCmd;
+import org.apache.cloudstack.api.Parameter;
+import org.apache.cloudstack.api.ServerApiException;
+import org.apache.cloudstack.api.response.NetworkOfferingResponse;
+
+import com.cloud.offering.NetworkOffering;
+
+@APICommand(name = "cloneNetworkOffering",
+ description = "Clones a network offering. All parameters are copied
from the source offering unless explicitly overridden. " +
+ "Use 'addServices' and 'dropServices' to modify the service
list without respecifying everything.",
+ responseObject = NetworkOfferingResponse.class,
+ requestHasSensitiveInfo = false,
+ responseHasSensitiveInfo = false,
+ since = "4.23.0")
+public class CloneNetworkOfferingCmd extends NetworkOfferingBaseCmd {
+
+ /////////////////////////////////////////////////////
+ //////////////// API parameters /////////////////////
+ /////////////////////////////////////////////////////
+
+ @Parameter(name = ApiConstants.SOURCE_OFFERING_ID,
+ type = BaseCmd.CommandType.UUID,
+ entityType = NetworkOfferingResponse.class,
+ required = true,
+ description = "The ID of the network offering to clone")
Review Comment:
in cloneBackupOffering, the description is `The ID of the source backup
offering to clone from`
may be consistent with other descriptions
##########
server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java:
##########
@@ -325,6 +326,79 @@ public BackupOffering importBackupOffering(final
ImportBackupOfferingCmd cmd) {
return savedOffering;
}
+ @Override
+ @ActionEvent(eventType = EventTypes.EVENT_VM_BACKUP_CLONE_OFFERING,
eventDescription = "cloning backup offering", create = true)
+ public BackupOffering cloneBackupOffering(final CloneBackupOfferingCmd
cmd) {
Review Comment:
the API has a parameter `zoneId` but it is never used here
```
@Parameter(name = ApiConstants.ZONE_ID, type = BaseCmd.CommandType.UUID,
entityType = ZoneResponse.class,
description = "The zone ID", required = false)
private Long zoneId;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
