From c698a3100420717d896d7d21d4ee151c47464493 Mon Sep 17 00:00:00 2001 From: Shubham Varshney Date: Tue, 7 May 2024 10:57:55 +0530 Subject: [PATCH] [PLAT-13800] skip setting imageBundle reference for YBM based clusters using machineImage Summary: YBM uses `machineImage` on the fly to specify the AMI to be used for deployment. Previously, YBA used to store the reference of defaultImageBundle in the universe for that case, which is falsy information & should not be done. This diff avoids setting the imageBundle reference in that case. Test Plan: Created a universe by specifying `machineImage` on the fly. Performed RemoveNode/AddNode Performed Remove/Release/AddNode operation. Performed OS Patching using the imageBundle on the above universe. Verified that the universe at this point - - Contains reference to the imageBundle - machineImage reference is set to `null` for the nodes. Reviewers: #yba-api-review!, amalyshev, dskorobogaty Reviewed By: amalyshev Subscribers: yugaware Differential Revision: https://phorge.dev.yugabyte.com/D34732 --- .../commissioner/tasks/UniverseTaskBase.java | 9 ++++++- .../subtasks/UpdateClusterUserIntent.java | 14 +++++++++++ .../com/yugabyte/yw/common/NodeManager.java | 25 ++++++++----------- .../yw/common/NodeUniverseManager.java | 5 +++- .../java/com/yugabyte/yw/common/Util.java | 9 ++++++- .../handlers/UniverseCRUDHandler.java | 4 ++- 6 files changed, 47 insertions(+), 19 deletions(-) diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/UniverseTaskBase.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/UniverseTaskBase.java index 9577b8a257f5..7e47428e0294 100644 --- a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/UniverseTaskBase.java +++ b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/UniverseTaskBase.java @@ -1914,7 +1914,10 @@ public SubTaskGroup createInstallNodeAgentTasks( universe.getUniverseDetails().getClusterByUuid(n.placementUuid); UUID imageBundleUUID = Util.retreiveImageBundleUUID( - universe.getUniverseDetails().arch, cluster.userIntent, provider); + universe.getUniverseDetails().arch, + cluster.userIntent, + provider, + confGetter.getStaticConf().getBoolean("yb.cloud.enabled")); if (imageBundleUUID != null) { ImageBundle.NodeProperties toOverwriteNodeProperties = imageBundleUtil.getNodePropertiesOrFail( @@ -1922,6 +1925,10 @@ public SubTaskGroup createInstallNodeAgentTasks( n.cloudInfo.region, cluster.userIntent.providerType.toString()); params.sshUser = toOverwriteNodeProperties.getSshUser(); + } else { + // ImageBundleUUID will be null for the case when YBM specifies machineImage + // on fly during universe creation. + params.sshUser = providerDetails.getSshUser(); } params.airgap = provider.getAirGapInstall(); diff --git a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/UpdateClusterUserIntent.java b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/UpdateClusterUserIntent.java index f615b0e2ef9a..f07e5fb22832 100644 --- a/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/UpdateClusterUserIntent.java +++ b/managed/src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/UpdateClusterUserIntent.java @@ -8,6 +8,7 @@ import java.util.UUID; import javax.inject.Inject; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.StringUtils; @Slf4j public class UpdateClusterUserIntent extends UniverseTaskBase { @@ -56,6 +57,19 @@ public void run(Universe universe) { // Update the imageBundle reference for the cluster in which node // is provisioned. cluster.userIntent.imageBundleUUID = taskParams().imageBundleUUID; + universeDetails.nodeDetailsSet.stream() + .forEach( + nodeDetail -> { + // YBM use case where the cluster would have been deployed using the + // machineImage + // but patched using the imageBundle. It will be good if clear the + // machineImage from + // nodeDetails. + if (nodeDetail.placementUuid.equals(cluster.uuid) + && StringUtils.isNotEmpty(nodeDetail.machineImage)) { + nodeDetail.machineImage = null; + } + }); } }); universe.setUniverseDetails(universeDetails); diff --git a/managed/src/main/java/com/yugabyte/yw/common/NodeManager.java b/managed/src/main/java/com/yugabyte/yw/common/NodeManager.java index 3338a8fc4f0c..f12cc45cd95f 100644 --- a/managed/src/main/java/com/yugabyte/yw/common/NodeManager.java +++ b/managed/src/main/java/com/yugabyte/yw/common/NodeManager.java @@ -227,8 +227,11 @@ private boolean imdsv2required(Architecture arch, UserIntent userIntent, Provide return false; } UUID imageBundleUUID = Util.retreiveImageBundleUUID(arch, userIntent, provider); - ImageBundle imageBundle = ImageBundle.get(imageBundleUUID); - return imageBundle.getDetails().useIMDSv2; + if (imageBundleUUID != null) { + ImageBundle imageBundle = ImageBundle.get(imageBundleUUID); + return imageBundle.getDetails().useIMDSv2; + } + return false; } private UserIntent getUserIntentFromParams(Universe universe, NodeTaskParams nodeTaskParam) { @@ -1799,8 +1802,10 @@ public ShellResponse nodeCommand(NodeCommandType type, NodeTaskParams nodeTaskPa List commandArgs = new ArrayList<>(); UserIntent userIntent = getUserIntentFromParams(nodeTaskParam); ImageBundle.NodeProperties toOverwriteNodeProperties = null; + Config config = this.runtimeConfigFactory.forProvider(provider); UUID imageBundleUUID = - Util.retreiveImageBundleUUID(arch, userIntent, nodeTaskParam.getProvider()); + Util.retreiveImageBundleUUID( + arch, userIntent, nodeTaskParam.getProvider(), config.getBoolean("yb.cloud.enabled")); if (imageBundleUUID != null) { Region region = nodeTaskParam.getRegion(); toOverwriteNodeProperties = @@ -1864,7 +1869,6 @@ public ShellResponse nodeCommand(NodeCommandType type, NodeTaskParams nodeTaskPa if (!(nodeTaskParam instanceof AnsibleCreateServer.Params)) { throw new RuntimeException("NodeTaskParams is not AnsibleCreateServer.Params"); } - Config config = this.runtimeConfigFactory.forProvider(provider); AnsibleCreateServer.Params taskParam = (AnsibleCreateServer.Params) nodeTaskParam; Common.CloudType cloudType = userIntent.providerType; if (!cloudType.equals(Common.CloudType.onprem)) { @@ -1968,11 +1972,7 @@ && imdsv2required(arch, userIntent, provider)) { // Backward compatiblity. imageBundleDefaultImage = taskParam.getRegion().getYbImage(); } - if (StringUtils.isNotBlank(taskParam.getMachineImage())) { - // YBM use case - in case machineImage is used for deploying the universe we should - // fallback to sshUser configured in the provider. - taskParam.sshUserOverride = provider.getDetails().getSshUser(); - } + String ybImage = Optional.ofNullable(taskParam.getMachineImage()).orElse(imageBundleDefaultImage); if (ybImage != null && !ybImage.isEmpty()) { @@ -2045,11 +2045,6 @@ && imdsv2required(arch, userIntent, provider)) { } else { imageBundleDefaultImage = taskParam.getRegion().getYbImage(); } - if (StringUtils.isNotBlank(taskParam.machineImage)) { - // YBM use case - in case machineImage is used for deploying the universe we should - // fallback to sshUser configured in the provider. - taskParam.sshUserOverride = provider.getDetails().getSshUser(); - } String ybImage = Optional.ofNullable(taskParam.machineImage).orElse(imageBundleDefaultImage); if (ybImage != null && !ybImage.isEmpty()) { @@ -2462,7 +2457,7 @@ && imdsv2required(arch, userIntent, provider)) { appendCertPathsToCheck(commandArgs, nodeTaskParam.getClientRootCA(), true, false); } - Config config = runtimeConfigFactory.forUniverse(universe); + config = runtimeConfigFactory.forUniverse(universe); SkipCertValidationType skipType = getSkipCertValidationType( diff --git a/managed/src/main/java/com/yugabyte/yw/common/NodeUniverseManager.java b/managed/src/main/java/com/yugabyte/yw/common/NodeUniverseManager.java index b11b8ec0ac73..380a7e7fa7ea 100644 --- a/managed/src/main/java/com/yugabyte/yw/common/NodeUniverseManager.java +++ b/managed/src/main/java/com/yugabyte/yw/common/NodeUniverseManager.java @@ -533,7 +533,10 @@ private void addConnectionParams( String sshPort = provider.getDetails().sshPort.toString(); UUID imageBundleUUID = Util.retreiveImageBundleUUID( - universe.getUniverseDetails().arch, cluster.userIntent, provider); + universe.getUniverseDetails().arch, + cluster.userIntent, + provider, + confGetter.getStaticConf().getBoolean("yb.cloud.enabled")); if (imageBundleUUID != null) { ImageBundle.NodeProperties toOverwriteNodeProperties = imageBundleUtil.getNodePropertiesOrFail( diff --git a/managed/src/main/java/com/yugabyte/yw/common/Util.java b/managed/src/main/java/com/yugabyte/yw/common/Util.java index 1acfa3fbc82e..669e6402c9d7 100644 --- a/managed/src/main/java/com/yugabyte/yw/common/Util.java +++ b/managed/src/main/java/com/yugabyte/yw/common/Util.java @@ -1196,10 +1196,17 @@ public static URL validateAndGetURL(String url, boolean defaultHttpScheme) { public static UUID retreiveImageBundleUUID( Architecture arch, UserIntent userIntent, Provider provider) { + return retreiveImageBundleUUID(arch, userIntent, provider, false); + } + + public static UUID retreiveImageBundleUUID( + Architecture arch, UserIntent userIntent, Provider provider, boolean cloudEnabled) { UUID imageBundleUUID = null; if (userIntent.imageBundleUUID != null) { imageBundleUUID = userIntent.imageBundleUUID; - } else if (provider.getUuid() != null) { + } else if (provider.getUuid() != null && !cloudEnabled) { + // Don't use defaultProvider bundle for YBM, as they will + // specify machineImage for provisioning the node. List bundles = ImageBundle.getDefaultForProvider(provider.getUuid()); if (bundles.size() > 0) { ImageBundle bundle = ImageBundleUtil.getDefaultBundleForUniverse(arch, bundles); diff --git a/managed/src/main/java/com/yugabyte/yw/controllers/handlers/UniverseCRUDHandler.java b/managed/src/main/java/com/yugabyte/yw/controllers/handlers/UniverseCRUDHandler.java index fc0bee101ae2..6fa481a75d34 100644 --- a/managed/src/main/java/com/yugabyte/yw/controllers/handlers/UniverseCRUDHandler.java +++ b/managed/src/main/java/com/yugabyte/yw/controllers/handlers/UniverseCRUDHandler.java @@ -556,7 +556,9 @@ public UniverseResp createUniverse(Customer customer, UniverseDefinitionTaskPara } validateRegionsAndZones(provider, c); // Configure the defaultimageBundle in case not specified. - if (c.userIntent.imageBundleUUID == null && provider.getCloudCode().imageBundleSupported()) { + if (c.userIntent.imageBundleUUID == null + && provider.getCloudCode().imageBundleSupported() + && !cloudEnabled) { if (provider.getImageBundles().size() > 0) { List bundles = ImageBundle.getDefaultForProvider(provider.getUuid()); if (bundles.size() > 0) {