Skip to content

Commit

Permalink
[PLAT-12850] Change order of task picked up on Operator Universe updates
Browse files Browse the repository at this point in the history 
Summary:
When multiple edits to Cr are done simultaneously, we saw that if disk resize
is not done first, it breaks the update flow. Changed the sequence to run Edit Universe( disk resize
) first.

Test Plan:
Manually verified
Added Unit tests covering scenarios of multiple edits where Edit Universe is picked first.
Added Unit tests for parsing KubernetesOverrides. The KubernetesOverrides which contains additional properties looks like this:

```
nodeSelector:
  label: "selector"
master: null
tserver: null
serviceEndpoints: null
resource:
  master:
    limits:
      cpu: 4
foo:
  bar: "1"
```

Reviewers: anijhawan

Reviewed By: anijhawan

Subscribers: yugaware

Differential Revision: https://phorge.dev.yugabyte.com/D32706
  • Loading branch information
@kv83821-yb
kv83821-yb committed Mar 6, 2024
1 parent ef7cc0c commit b1319c0
Show file tree
Hide file tree
Showing 3 changed files with 158 additions and 25 deletions.
62 changes: 41 additions & 21 deletions managed/src/main/java/com/yugabyte/yw/common/operator/YBUniverseReconciler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.databind.module.SimpleModule;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
import com.google.common.annotations.VisibleForTesting;
import com.yugabyte.yw.commissioner.Common.CloudType;
Expand All @@ -19,6 +20,7 @@
import com.yugabyte.yw.common.gflags.SpecificGFlags;
import com.yugabyte.yw.common.gflags.SpecificGFlags.PerProcessFlags;
import com.yugabyte.yw.common.operator.OperatorStatusUpdater.UniverseState;
import com.yugabyte.yw.common.operator.helpers.KubernetesOverridesSerializer;
import com.yugabyte.yw.common.operator.utils.KubernetesEnvironmentVariables;
import com.yugabyte.yw.common.operator.utils.OperatorUtils;
import com.yugabyte.yw.common.operator.utils.OperatorWorkQueue;
Expand Down Expand Up @@ -55,10 +57,10 @@
import io.fabric8.kubernetes.api.model.Secret;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.dsl.MixedOperation;
import io.fabric8.kubernetes.client.dsl.Resource;
import io.fabric8.kubernetes.client.informers.SharedIndexInformer;
import io.fabric8.kubernetes.client.informers.cache.Lister;
import io.yugabyte.operator.v1alpha1.YBUniverse;
import io.yugabyte.operator.v1alpha1.ybuniversespec.KubernetesOverrides;
import io.yugabyte.operator.v1alpha1.ybuniversespec.YcqlPassword;
import io.yugabyte.operator.v1alpha1.ybuniversespec.YsqlPassword;
import java.util.Base64;
Expand Down Expand Up @@ -87,7 +89,12 @@ public class YBUniverseReconciler extends AbstractReconciler<YBUniverse> {
private final SharedIndexInformer<YBUniverse> ybUniverseInformer;
private final String namespace;
private final Lister<YBUniverse> ybUniverseLister;
private final MixedOperation<YBUniverse, KubernetesResourceList<YBUniverse>, Resource<YBUniverse>>
// Resource here has full class name since it conflicts with
// ybuniversespec.kubernetesoverrides.Resource
private final MixedOperation<
YBUniverse,
KubernetesResourceList<YBUniverse>,
io.fabric8.kubernetes.client.dsl.Resource<YBUniverse>>
ybUniverseClient;
public static final String APP_LABEL = "app";
private final UniverseCRUDHandler universeCRUDHandler;
Expand Down Expand Up @@ -685,7 +692,8 @@ private Result createUniverse(
}
}

private void editUniverse(Customer cust, Universe universe, YBUniverse ybUniverse) {
@VisibleForTesting
protected void editUniverse(Customer cust, Universe universe, YBUniverse ybUniverse) {
UniverseDefinitionTaskParams universeDetails = universe.getUniverseDetails();
if (universeDetails == null || universeDetails.getPrimaryCluster() == null) {
throw new RuntimeException(
Expand Down Expand Up @@ -713,7 +721,18 @@ private void editUniverse(Customer cust, Universe universe, YBUniverse ybUnivers
UUID taskUUID = null;

try {
if (!incomingIntent.universeOverrides.equals(currentUserIntent.universeOverrides)) {
if (shouldUpdateYbUniverse(currentUserIntent, incomingIntent)) {
log.info("Calling Edit Universe");
kubernetesStatusUpdater.createYBUniverseEventStatus(
universe, k8ResourceDetails, TaskType.EditKubernetesUniverse.name());
if (checkAndHandleUniverseLock(
ybUniverse, universe, OperatorWorkQueue.ResourceAction.NO_OP)) {
return;
}
kubernetesStatusUpdater.updateUniverseState(k8ResourceDetails, UniverseState.EDITING);
taskUUID = updateYBUniverse(universeDetails, cust, ybUniverse);
} else if (!StringUtils.equals(
incomingIntent.universeOverrides, currentUserIntent.universeOverrides)) {
log.info("Updating Kubernetes Overrides");
kubernetesStatusUpdater.createYBUniverseEventStatus(
universe, k8ResourceDetails, TaskType.KubernetesOverridesUpgrade.name());
Expand All @@ -735,16 +754,6 @@ private void editUniverse(Customer cust, Universe universe, YBUniverse ybUnivers
}
kubernetesStatusUpdater.updateUniverseState(k8ResourceDetails, UniverseState.EDITING);
taskUUID = updateGflagsYbUniverse(universeDetails, cust, ybUniverse);
} else if (shouldUpdateYbUniverse(currentUserIntent, incomingIntent)) {
log.info("Calling Edit Universe");
kubernetesStatusUpdater.createYBUniverseEventStatus(
universe, k8ResourceDetails, TaskType.EditKubernetesUniverse.name());
if (checkAndHandleUniverseLock(
ybUniverse, universe, OperatorWorkQueue.ResourceAction.NO_OP)) {
return;
}
kubernetesStatusUpdater.updateUniverseState(k8ResourceDetails, UniverseState.EDITING);
taskUUID = updateYBUniverse(universeDetails, cust, ybUniverse);
} else if (!currentUserIntent.ybSoftwareVersion.equals(incomingIntent.ybSoftwareVersion)) {
log.info("Upgrading software");
kubernetesStatusUpdater.createYBUniverseEventStatus(
Expand Down Expand Up @@ -908,18 +917,29 @@ protected UniverseConfigureTaskParams createTaskParams(YBUniverse ybUniverse, UU
return taskParams;
}

@VisibleForTesting
protected String getKubernetesOverridesString(KubernetesOverrides kubernetesOverrides) {
ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
mapper.setSerializationInclusion(Include.NON_NULL);
mapper.setSerializationInclusion(Include.NON_EMPTY);
SimpleModule simpleModule = new SimpleModule();
simpleModule.addSerializer(new KubernetesOverridesSerializer());
mapper.registerModule(simpleModule);
try {
return mapper.writeValueAsString(kubernetesOverrides);
} catch (Exception e) {
log.error("Unable to parse universe overrides", e);
}
return null;
}

private UserIntent createUserIntent(YBUniverse ybUniverse, UUID customerUUID, boolean isCreate) {
try {
UserIntent userIntent = new UserIntent();
userIntent.universeName = getYbaUniverseName(ybUniverse);
ObjectMapper mapper = new ObjectMapper(new YAMLFactory());
mapper.setSerializationInclusion(Include.NON_NULL);
mapper.setSerializationInclusion(Include.NON_EMPTY);
try {
if (ybUniverse.getSpec().getKubernetesOverrides() != null) {
userIntent.universeOverrides =
mapper.writeValueAsString(ybUniverse.getSpec().getKubernetesOverrides());
} catch (Exception e) {
log.error("Unable to parse universe overrides", e);
getKubernetesOverridesString(ybUniverse.getSpec().getKubernetesOverrides());
}
Provider provider = getProvider(customerUUID, ybUniverse);
userIntent.provider = provider.getUuid().toString();
Expand Down
34 changes: 34 additions & 0 deletions .../src/main/java/com/yugabyte/yw/common/operator/helpers/KubernetesOverridesSerializer.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package com.yugabyte.yw.common.operator.helpers;

import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.SerializerProvider;
import com.fasterxml.jackson.databind.ser.std.StdSerializer;
import io.yugabyte.operator.v1alpha1.ybuniversespec.KubernetesOverrides;
import io.yugabyte.operator.v1alpha1.ybuniversespec.kubernetesoverrides.Resource;
import java.io.IOException;
import java.util.Map;

public class KubernetesOverridesSerializer extends StdSerializer<KubernetesOverrides> {

public KubernetesOverridesSerializer() {
super(KubernetesOverrides.class);
}

@Override
public void serialize(KubernetesOverrides value, JsonGenerator gen, SerializerProvider provider)
throws IOException {
Resource resource = value.getResource();
gen.writeStartObject();
provider.defaultSerializeField("nodeSelector", value.getNodeSelector(), gen);
provider.defaultSerializeField("master", value.getMaster(), gen);
provider.defaultSerializeField("tserver", value.getTserver(), gen);
provider.defaultSerializeField("serviceEndpoints", value.getServiceEndpoints(), gen);
provider.defaultSerializeField("resource", resource, gen);
if (value.getAdditionalProperties() != null) {
for (Map.Entry<String, Object> entry : value.getAdditionalProperties().entrySet()) {
gen.writeObjectField(entry.getKey(), entry.getValue());
}
}
gen.writeEndObject();
}
}
87 changes: 83 additions & 4 deletions managed/src/test/java/com/yugabyte/yw/common/operator/YBUniverseReconcilerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,20 +32,26 @@
import com.yugabyte.yw.models.Universe;
import com.yugabyte.yw.models.Users;
import com.yugabyte.yw.models.helpers.TaskType;
import io.fabric8.kubernetes.api.model.IntOrString;
import io.fabric8.kubernetes.api.model.KubernetesResourceList;
import io.fabric8.kubernetes.api.model.ObjectMeta;
import io.fabric8.kubernetes.client.KubernetesClient;
import io.fabric8.kubernetes.client.dsl.MixedOperation;
import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation;
import io.fabric8.kubernetes.client.dsl.Resource;
import io.fabric8.kubernetes.client.informers.SharedIndexInformer;
import io.fabric8.kubernetes.client.informers.cache.Indexer;
import io.yugabyte.operator.v1alpha1.YBUniverse;
import io.yugabyte.operator.v1alpha1.YBUniverseSpec;
import io.yugabyte.operator.v1alpha1.YBUniverseStatus;
import io.yugabyte.operator.v1alpha1.ybuniversespec.DeviceInfo;
import io.yugabyte.operator.v1alpha1.ybuniversespec.KubernetesOverrides;
import io.yugabyte.operator.v1alpha1.ybuniversespec.kubernetesoverrides.Resource;
import io.yugabyte.operator.v1alpha1.ybuniversespec.kubernetesoverrides.resource.Master;
import io.yugabyte.operator.v1alpha1.ybuniversespec.kubernetesoverrides.resource.master.Limits;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import javax.annotation.Nullable;
import lombok.extern.slf4j.Slf4j;
Expand All @@ -66,14 +72,20 @@ public class YBUniverseReconcilerTest extends FakeDBApplication {
@Mock KubernetesClient client;

@Mock
MixedOperation<YBUniverse, KubernetesResourceList<YBUniverse>, Resource<YBUniverse>>
MixedOperation<
YBUniverse,
KubernetesResourceList<YBUniverse>,
io.fabric8.kubernetes.client.dsl.Resource<YBUniverse>>
ybUniverseClient;

@Mock
NonNamespaceOperation<YBUniverse, KubernetesResourceList<YBUniverse>, Resource<YBUniverse>>
NonNamespaceOperation<
YBUniverse,
KubernetesResourceList<YBUniverse>,
io.fabric8.kubernetes.client.dsl.Resource<YBUniverse>>
inNamespaceYBUClient;

@Mock Resource<YBUniverse> ybUniverseResource;
@Mock io.fabric8.kubernetes.client.dsl.Resource<YBUniverse> ybUniverseResource;
@Mock RuntimeConfGetter confGetter;
@Mock RuntimeConfGetter confGetterForOperatorUtils;
@Mock YBInformerFactory informerFactory;
Expand Down Expand Up @@ -374,6 +386,72 @@ public void testCreateAutoProviderFailStatusUpdate() throws Exception {
any(KubernetesResourceDetails.class), eq(UniverseState.ERROR_CREATING));
}

@Test
public void testMultipleSpecUpdatePickEditFirst() throws Exception {
String universeName = "test-multiple-spec-updates";
YBUniverse ybUniverse = createYbUniverse(universeName);
UniverseDefinitionTaskParams taskParams =
ybUniverseReconciler.createTaskParams(ybUniverse, defaultCustomer.getUuid());
Universe oldUniverse = Universe.create(taskParams, defaultCustomer.getId());

// Update spec
ybUniverse.getSpec().getDeviceInfo().setVolumeSize(20L);
KubernetesOverrides ko = new KubernetesOverrides();
Map<String, String> nodeSelectorMap = new HashMap<>();
nodeSelectorMap.put("foo", "bar");
ko.setNodeSelector(nodeSelectorMap);
ybUniverse.getSpec().setKubernetesOverrides(ko);

// Call edit
ybUniverseReconciler.editUniverse(defaultCustomer, oldUniverse, ybUniverse);
// Verify update is called
ArgumentCaptor<UniverseDefinitionTaskParams> uDTCaptor =
ArgumentCaptor.forClass(UniverseDefinitionTaskParams.class);
Mockito.verify(universeCRUDHandler, Mockito.times(1))
.update(any(Customer.class), any(Universe.class), uDTCaptor.capture());
assertTrue(uDTCaptor.getValue().getPrimaryCluster().userIntent.deviceInfo.volumeSize == 20L);
// Verify upgrade handler is not called
Mockito.verifyNoInteractions(upgradeUniverseHandler);
}

@Test
public void testParseKubernetesOverridesNoAdditionalProperty() {
KubernetesOverrides overrides = createKubernetesOverrides();

String overridesString = ybUniverseReconciler.getKubernetesOverridesString(overrides);
assertTrue(overridesString.length() > 0);
}

@Test
public void testParseKubernetesOverridesWithAdditionalProperty() {
KubernetesOverrides overrides = createKubernetesOverrides();

Map<String, Object> additionalPropertiesMap = new HashMap<>();
additionalPropertiesMap.put("foo", "bar");
overrides.setAdditionalProperties(additionalPropertiesMap);

String overridesString = ybUniverseReconciler.getKubernetesOverridesString(overrides);
assertTrue(overridesString.length() > 0);
assertTrue(overridesString.contains("foo") && overridesString.contains("bar"));
}

private KubernetesOverrides createKubernetesOverrides() {
KubernetesOverrides overrides = new KubernetesOverrides();
Map<String, String> nodeSelectorMap = new HashMap<>();
nodeSelectorMap.put("label", "selector");
overrides.setNodeSelector(nodeSelectorMap);

Resource resource = new Resource();
Limits limit = new Limits();
limit.setCpu(new IntOrString(new Integer(4)));
Master masterResource = new Master();
masterResource.setLimits(limit);
resource.setMaster(masterResource);
overrides.setResource(resource);

return overrides;
}

private YBUniverse createYbUniverse() {
return createYbUniverse(null);
}
Expand Down Expand Up @@ -402,6 +480,7 @@ private YBUniverse createYbUniverse(@Nullable String univName) {
spec.setYcqlPassword(null);
spec.setProviderName(defaultProvider.getName());
DeviceInfo deviceInfo = new DeviceInfo();
deviceInfo.setVolumeSize(10L);
spec.setDeviceInfo(deviceInfo);

universe.setMetadata(metadata);
Expand Down

0 comments on commit b1319c0

Please sign in to comment.