Skip to content

Commit

Permalink
[PLAT-13800] skip setting imageBundle reference for YBM based cluster…
Browse files Browse the repository at this point in the history
…s 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
  • Loading branch information
Vars-07 committed May 7, 2024
1 parent b21d37c commit 53aafba
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1914,14 +1914,21 @@ 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(
imageBundleUUID,
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
25 changes: 10 additions & 15 deletions managed/src/main/java/com/yugabyte/yw/common/NodeManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -1799,8 +1802,10 @@ public ShellResponse nodeCommand(NodeCommandType type, NodeTaskParams nodeTaskPa
List<String> 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 =
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
9 changes: 8 additions & 1 deletion managed/src/main/java/com/yugabyte/yw/common/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<ImageBundle> bundles = ImageBundle.getDefaultForProvider(provider.getUuid());
if (bundles.size() > 0) {
ImageBundle bundle = ImageBundleUtil.getDefaultBundleForUniverse(arch, bundles);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<ImageBundle> bundles = ImageBundle.getDefaultForProvider(provider.getUuid());
if (bundles.size() > 0) {
Expand Down

0 comments on commit 53aafba

Please sign in to comment.