Skip to content

Commit 92c62e9

Browse files
vikyathharekalvikyathharekal
authored
feat: [CDS-40986]: GA FF KEEP_PT_AFTER_K8S_DOWNSCALE (#36475)
* [CDS-40986]: GA FF KEEP_PT_AFTER_K8S_DOWNSCALE * [CDS-40986]: Fix checkstyle * [CDS-40986]: Fix tests * [CDS-41821]: Fix retryable bool * [CDS-40986]: add is PERPETUAL_TASK flow check before throwing exception
1 parent 431180a commit 92c62e9

4 files changed

Lines changed: 19 additions & 63 deletions

File tree

‎390-db-migration/src/main/java/io/harness/migrations/all/UpdateCorruptedInstanceStatsMigration.java‎

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,13 @@
88
package io.harness.migrations.all;
99

1010
import static io.harness.persistence.HQuery.excludeAuthority;
11-
import static io.harness.threading.Morpheus.sleep;
1211

1312
import static java.lang.String.format;
1413
import static java.util.Arrays.asList;
1514
import static org.apache.commons.lang3.StringUtils.isBlank;
1615

1716
import io.harness.annotations.dev.HarnessTeam;
1817
import io.harness.annotations.dev.OwnedBy;
19-
import io.harness.beans.FeatureName;
2018
import io.harness.event.timeseries.processor.EventProcessor;
2119
import io.harness.event.usagemetrics.UsageMetricsEventPublisher;
2220
import io.harness.ff.FeatureFlagService;
@@ -29,7 +27,6 @@
2927
import software.wings.beans.InfrastructureMapping;
3028
import software.wings.beans.InfrastructureMapping.InfrastructureMappingKeys;
3129
import software.wings.beans.infrastructure.instance.Instance;
32-
import software.wings.beans.infrastructure.instance.Instance.InstanceKeys;
3330
import software.wings.beans.infrastructure.instance.stats.InstanceStatsSnapshot;
3431
import software.wings.beans.infrastructure.instance.stats.InstanceStatsSnapshot.AggregateCount;
3532
import software.wings.beans.infrastructure.instance.stats.InstanceStatsSnapshot.InstanceStatsSnapshotKeys;
@@ -47,7 +44,6 @@
4744
import java.sql.ResultSet;
4845
import java.sql.SQLException;
4946
import java.sql.Timestamp;
50-
import java.time.Duration;
5147
import java.time.Instant;
5248
import java.util.ArrayList;
5349
import java.util.Collection;
@@ -116,36 +112,12 @@ private Set<String> deleteDuplicateInstanceMigration(Set<String> firstReleaseCon
116112
.project(InfrastructureMappingKeys.appId, true)
117113
.disableValidation()
118114
.fetch())) {
119-
int updated = 0;
120-
BulkWriteOperation instanceWriteOperation =
121-
mongoPersistence.getCollection(Instance.class).initializeUnorderedBulkOperation();
122115
while (infraMappingIter.hasNext()) {
123116
InfrastructureMapping infraMapping = infraMappingIter.next();
124117
affectedAccounts.add(infraMapping.getAccountId());
125-
if (featureFlagService.isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, infraMapping.getAccountId())) {
126-
continue;
127-
}
128-
129-
HashMap<String, Object> query = new HashMap<>();
130-
query.put(InstanceKeys.appId, infraMapping.getAppId());
131-
query.put(InstanceKeys.infraMappingId, infraMapping.getUuid());
132-
query.put(InstanceKeys.createdAt, new BasicDBObject(ImmutableMap.of("$gte", FROM, "$lt", UNTIL)));
133-
query.put(InstanceKeys.lastWorkflowExecutionId, null);
134-
query.put(InstanceKeys.lastDeployedById, "AUTO_SCALE");
135-
136-
instanceWriteOperation.find(new BasicDBObject(query)).remove();
137-
updated++;
138-
139-
if (updated >= INFRA_INSTANCE_BATCH_SIZE) {
140-
log.info("Delete corrupted instances for {} infra mappings: {}", updated, instanceWriteOperation.execute());
141-
sleep(Duration.ofMillis(100)); // give some relax time
142-
instanceWriteOperation = mongoPersistence.getCollection(Instance.class).initializeUnorderedBulkOperation();
143-
updated = 0;
144-
}
145-
}
146-
147-
if (updated != 0) {
148-
log.info("Delete corrupted instances for {} infra mappings: {}", updated, instanceWriteOperation.execute());
118+
// Since the FF KEEP_PT_AFTER_K8S_DOWNSCALE is going to be GA,
119+
// none of the instances will be picked up for deletion.
120+
// We would still return the affected accounts so that it can be used to update the corrupted instance stats.
149121
}
150122
}
151123

‎400-rest/src/main/java/software/wings/service/impl/instance/ContainerInstanceHandler.java‎

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -172,9 +172,6 @@ private void syncInstancesInternal(ContainerInfrastructureMapping containerInfra
172172
return;
173173
}
174174

175-
boolean keepPTAfterDownScale = instanceSyncFlow == PERPETUAL_TASK
176-
&& featureFlagService.isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, containerInfraMapping.getAccountId());
177-
178175
if (instanceSyncFlow == PERPETUAL_TASK && responseData != null) {
179176
ContainerMetadata perpetualTaskMetadata = getContainerMetadataFromInstanceSyncResponse(responseData);
180177

@@ -189,7 +186,7 @@ private void syncInstancesInternal(ContainerInfrastructureMapping containerInfra
189186
// Current logic is to catch any exception and if there is no successful sync status during 7 days then delete
190187
// all infra perpetual tasks. We're exploiting this to handle the case when replica is scaled down to 0 and
191188
// after n time is scaled back, so we will delete perpetual task only if after 7 days there are no pods
192-
if (keepPTAfterDownScale && isEmpty(syncResponse.getK8sPodInfoList())) {
189+
if (isEmpty(syncResponse.getK8sPodInfoList())) {
193190
// In this case there is nothing to be processed since there is no instances in db that need to be removed
194191
log.info("Still there is no pods found for [app: {}, namespace: {}, release name: {}]", appId,
195192
syncResponse.getNamespace(), syncResponse.getReleaseName());
@@ -201,7 +198,7 @@ private void syncInstancesInternal(ContainerInfrastructureMapping containerInfra
201198
syncResponse.getK8sPodInfoList());
202199
return;
203200

204-
} else if (responseData instanceof ContainerSyncResponse && keepPTAfterDownScale) {
201+
} else if (responseData instanceof ContainerSyncResponse) {
205202
ContainerSyncResponse syncResponse = (ContainerSyncResponse) responseData;
206203

207204
// Current logic is to catch any exception and if there is no successful sync status during 7 days then delete
@@ -257,7 +254,7 @@ private void syncInstancesInternal(ContainerInfrastructureMapping containerInfra
257254
// delete all infra perpetual tasks. We're exploiting this to handle the case when replica is scaled down
258255
// to 0 and after n time is scaled back, so we will delete perpetual task only if after 7 days there are
259256
// no pods
260-
if (keepPTAfterDownScale && isEmpty(syncResponse.getK8sPodInfoList())) {
257+
if (isEmpty(syncResponse.getK8sPodInfoList())) {
261258
log.info("No pods found for [app: {}, namespace: {}, release name: {}]", appId,
262259
syncResponse.getNamespace(), syncResponse.getReleaseName());
263260
noInstancesException =
@@ -291,7 +288,7 @@ private void syncInstancesInternal(ContainerInfrastructureMapping containerInfra
291288
// delete all infra perpetual tasks. We're exploiting this to handle the case when replica is scaled down to
292289
// 0 and after n time is scaled back, so we will delete perpetual task only if after 7 days there are no
293290
// containers
294-
if (keepPTAfterDownScale && isEmpty(syncResponse.getContainerInfoList())) {
291+
if (isEmpty(syncResponse.getContainerInfoList())) {
295292
log.info("No containers found for [app: {}, namespace: {}, release name: {}]", appId,
296293
syncResponse.getNamespace(), syncResponse.getReleaseName());
297294

@@ -304,7 +301,7 @@ private void syncInstancesInternal(ContainerInfrastructureMapping containerInfra
304301
}
305302
}
306303

307-
if (keepPTAfterDownScale && noInstancesException != null) {
304+
if (instanceSyncFlow == PERPETUAL_TASK && noInstancesException != null) {
308305
throw noInstancesException;
309306
}
310307
}
@@ -1574,30 +1571,26 @@ public Status getStatus(InfrastructureMapping infrastructureMapping, DelegateRes
15741571
}
15751572

15761573
return response instanceof K8sTaskExecutionResponse
1577-
? getK8sPerpetualTaskStatus((K8sTaskExecutionResponse) response, infrastructureMapping.getAccountId())
1578-
: getContainerSyncPerpetualTaskStatus((ContainerSyncResponse) response, infrastructureMapping.getAccountId());
1574+
? getK8sPerpetualTaskStatus((K8sTaskExecutionResponse) response)
1575+
: getContainerSyncPerpetualTaskStatus((ContainerSyncResponse) response);
15791576
}
15801577

1581-
private Status getK8sPerpetualTaskStatus(K8sTaskExecutionResponse response, String accountId) {
1578+
private Status getK8sPerpetualTaskStatus(K8sTaskExecutionResponse response) {
15821579
boolean success = response.getCommandExecutionStatus() == SUCCESS;
1583-
K8sInstanceSyncResponse k8sInstanceSyncResponse = (K8sInstanceSyncResponse) response.getK8sTaskResponse();
1584-
boolean keepPTAfterDownscale = featureFlagService.isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, accountId);
1585-
boolean deleteTask = !keepPTAfterDownscale && success && isEmpty(k8sInstanceSyncResponse.getK8sPodInfoList());
15861580
String errorMessage = success ? null : response.getErrorMessage();
15871581

1588-
return Status.builder().retryable(!deleteTask).errorMessage(errorMessage).success(success).build();
1582+
return Status.builder().retryable(true).errorMessage(errorMessage).success(success).build();
15891583
}
15901584

1591-
private Status getContainerSyncPerpetualTaskStatus(ContainerSyncResponse response, String accountId) {
1585+
private Status getContainerSyncPerpetualTaskStatus(ContainerSyncResponse response) {
15921586
boolean success = response.getCommandExecutionStatus() == SUCCESS;
15931587
boolean deleteTask;
15941588
if (response.isEcs()) {
15951589
// Ecs
15961590
deleteTask = success && !response.isEcsServiceExists();
15971591
} else {
15981592
// K8s v1
1599-
boolean keepPTAfterDownscale = featureFlagService.isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, accountId);
1600-
deleteTask = !keepPTAfterDownscale && success && isEmpty(response.getContainerInfoList());
1593+
deleteTask = false;
16011594
}
16021595

16031596
String errorMessage = success ? null : response.getErrorMessage();

‎400-rest/src/test/java/software/wings/service/impl/instance/ContainerInstanceHandlerTest.java‎

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@
6969
import io.harness.annotations.dev.TargetModule;
7070
import io.harness.beans.ArtifactMetadata;
7171
import io.harness.beans.EnvironmentType;
72-
import io.harness.beans.FeatureName;
7372
import io.harness.beans.PageResponse;
7473
import io.harness.category.element.UnitTests;
7574
import io.harness.container.ContainerInfo;
@@ -193,8 +192,6 @@ public void setUp() {
193192
.get(any(), any(), anyBoolean());
194193

195194
doReturn(Service.builder().name(SERVICE_NAME).build()).when(serviceResourceService).getWithDetails(any(), any());
196-
197-
doReturn(false).when(featureFlagService).isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, ACCOUNT_ID);
198195
}
199196

200197
private InfrastructureMapping getInframapping(String inframappingType) {
@@ -1781,9 +1778,10 @@ public void shouldUpdateAllIfWorkloadNameIsEmpty() {
17811778
.build())
17821779
.build());
17831780

1781+
EcsContainerInfo containerInfo = Builder.anEcsContainerInfo().build();
17841782
ContainerSyncResponse responseData;
17851783
responseData = ContainerSyncResponse.builder()
1786-
.containerInfoList(Collections.emptyList())
1784+
.containerInfoList(Collections.singletonList(containerInfo))
17871785
.commandExecutionStatus(CommandExecutionStatus.SUCCESS)
17881786
.build();
17891787
ContainerInfrastructureMapping infrastructureMapping;
@@ -1824,9 +1822,10 @@ public void shouldUpdateInstancesWithMatchingWorkloadName() {
18241822
.build())
18251823
.build());
18261824

1825+
EcsContainerInfo containerInfo = Builder.anEcsContainerInfo().build();
18271826
ContainerSyncResponse responseData;
18281827
responseData = ContainerSyncResponse.builder()
1829-
.containerInfoList(Collections.emptyList())
1828+
.containerInfoList(Collections.singletonList(containerInfo))
18301829
.controllerName("controllerName:0")
18311830
.commandExecutionStatus(CommandExecutionStatus.SUCCESS)
18321831
.build();
@@ -1840,7 +1839,7 @@ public void shouldUpdateInstancesWithMatchingWorkloadName() {
18401839
verify(instanceService, never()).delete(Sets.newHashSet(INSTANCE_2_ID));
18411840

18421841
responseData = ContainerSyncResponse.builder()
1843-
.containerInfoList(Collections.emptyList())
1842+
.containerInfoList(Collections.singletonList(containerInfo))
18441843
.controllerName("controllerName:1")
18451844
.commandExecutionStatus(CommandExecutionStatus.SUCCESS)
18461845
.build();
@@ -2102,8 +2101,6 @@ public void shouldUpdateInstancesFromPerpetualTaskResponseControllerNameIsEmpty(
21022101
createKubernetesContainerInstance("instance4", "releaseY", "namespaceX", null),
21032102
createKubernetesContainerInstance("instance5", "releaseY", "namespaceY", null));
21042103

2105-
doReturn(true).when(featureFlagService).isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, ACCOUNT_ID);
2106-
21072104
// Ref: ContainerInstanceSyncPerpetualTaskClient#getPerpetualTaskData at 207, controllerName will be always empty
21082105
// string when the actual value is null
21092106
ContainerSyncResponse instanceSyncResponse =
@@ -2327,7 +2324,6 @@ public void shouldThrowNoInstancesExceptionInstancesExistsInDb() {
23272324
createK8sPodInstance("instance2", "releaseX", "namespaceX"));
23282325

23292326
K8sInstanceSyncResponse instanceSyncResponse = creteK8sPodSyncResponseWith("releaseX", "namespaceX");
2330-
doReturn(true).when(featureFlagService).isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, ACCOUNT_ID);
23312327
assertThatThrownBy(()
23322328
-> assertSavedAndDeletedInstances(
23332329
instancesInDb, instanceSyncResponse, emptyList(), asList("instance1", "instance2")))
@@ -2339,7 +2335,6 @@ instancesInDb, instanceSyncResponse, emptyList(), asList("instance1", "instance2
23392335
@Category(UnitTests.class)
23402336
public void shouldThrowNoInstancesExceptionNOInstancesExistsInDb() {
23412337
K8sInstanceSyncResponse instanceSyncResponse = creteK8sPodSyncResponseWith("releaseX", "namespaceX");
2342-
doReturn(true).when(featureFlagService).isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, ACCOUNT_ID);
23432338

23442339
assertThatThrownBy(
23452340
() -> assertSavedAndDeletedInstances(emptyList(), instanceSyncResponse, emptyList(), emptyList()))
@@ -2358,7 +2353,6 @@ public void shouldThrowNoInstancesExceptionForKubernetesContainerDeployment() {
23582353
.isEcs(false)
23592354
.commandExecutionStatus(CommandExecutionStatus.SUCCESS)
23602355
.build();
2361-
doReturn(true).when(featureFlagService).isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, ACCOUNT_ID);
23622356

23632357
assertThatThrownBy(() -> assertSavedAndDeletedInstances(instancesInDb, syncResponse, emptyList(), emptyList()))
23642358
.isInstanceOf(NoInstancesException.class);
@@ -2369,7 +2363,6 @@ public void shouldThrowNoInstancesExceptionForKubernetesContainerDeployment() {
23692363
@Category(UnitTests.class)
23702364
public void shouldThrowNoInstancesExceptionForKubernetesContainerDeploymentNoInstancesInDb() {
23712365
ContainerSyncResponse syncResponse = createContainerSyncResponseWith("release-name", "default", "controller");
2372-
doReturn(true).when(featureFlagService).isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, ACCOUNT_ID);
23732366

23742367
assertThatThrownBy(() -> assertSavedAndDeletedInstances(emptyList(), syncResponse, emptyList(), emptyList()))
23752368
.isInstanceOf(NoInstancesException.class);
@@ -2381,7 +2374,6 @@ public void shouldThrowNoInstancesExceptionForKubernetesContainerDeploymentNoIns
23812374
public void shouldAddInstancesFromContainerSyncEvenNoInstancesInDb() {
23822375
ContainerSyncResponse syncResponse =
23832376
createContainerSyncResponseWith("release-name", "default", "controller", "instance-1", "instance-2");
2384-
doReturn(true).when(featureFlagService).isEnabled(FeatureName.KEEP_PT_AFTER_K8S_DOWNSCALE, ACCOUNT_ID);
23852377

23862378
assertSavedAndDeletedInstances(emptyList(), syncResponse, asList("instance-1", "instance-2"), emptyList());
23872379
}

‎956-feature-flag-beans/src/main/java/io/harness/beans/FeatureName.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,6 @@ public enum FeatureName {
263263
USE_ANALYTIC_MONGO_FOR_GRAPHQL_QUERY,
264264
DYNATRACE_APM_ENABLED,
265265
CUSTOM_POLICY_STEP,
266-
KEEP_PT_AFTER_K8S_DOWNSCALE,
267266
CCM_AS_DRY_RUN("Dry Run functionality of the AutoStopping Rules", HarnessTeam.CE),
268267
DONT_RESTRICT_PARALLEL_STAGE_COUNT,
269268
NG_EXECUTION_INPUT,

0 commit comments

Comments
 (0)