Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#P39046999 Persist last (5) successful local deployments as part of CLI application code #164

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import com.aws.greengrass.authorization.Permission;
import com.aws.greengrass.authorization.exceptions.AuthorizationException;
import com.aws.greengrass.componentmanager.ComponentStore;
import com.aws.greengrass.config.Node;
import com.aws.greengrass.config.Topic;
import com.aws.greengrass.config.Topics;
import com.aws.greengrass.deployment.DeploymentQueue;
import com.aws.greengrass.deployment.model.ConfigurationUpdateOperation;
Expand Down Expand Up @@ -78,10 +80,12 @@
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collection;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.Collectors;
import javax.inject.Inject;

Expand Down Expand Up @@ -118,6 +122,7 @@ public class CLIEventStreamAgent {
private static final int DEBUG_PASSWORD_LENGTH_REQUIREMENT = 32;
private static final String DEBUG_USERNAME = "debug";
private static final Duration DEBUG_PASSWORD_EXPIRATION = Duration.ofHours(8);
private static final int PERSIST_LIMIT = 5;
protected static final String CERT_FINGERPRINT_NAMESPACE = "_certificateFingerprint";

@Inject
Expand Down Expand Up @@ -179,7 +184,27 @@ public void persistLocalDeployment(Topics serviceConfig, Map<String, Object> dep
String deploymentId = (String) deploymentDetails.get(DEPLOYMENT_ID_KEY_NAME);
Topics localDeploymentDetails = localDeployments.lookupTopics(deploymentId);
localDeploymentDetails.replaceAndWait(deploymentDetails);
// TODO: [P41178971]: Implement a limit on no of local deployments to persist status for
// #P39046999 Persist last (5) successful local deployments as part of CLI application code.
if (localDeployments.size() > PERSIST_LIMIT) {
List<Topics> childrenToRemove = new ArrayList<>();
AtomicInteger deploymentSucceeded = new AtomicInteger();
localDeployments.forEach(topics -> {
Topics deploymentTopics = (Topics) topics;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check using instanceof before blindly casting to Topics.

Topic existingChild = deploymentTopics.find(DEPLOYMENT_STATUS_KEY_NAME);
String value = existingChild.getOnce().toString();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use Coerce.toString(deploymentTopics.find(DEPLOYMENT_STATUS_KEY_NAME)

if (DeploymentStatus.SUCCEEDED.toString().equals(value)) {
deploymentSucceeded.getAndIncrement();
childrenToRemove.add(deploymentTopics);
}
});
// if more than PERSIST_LIMIT deployments are success, remove until PERSIST_LIMIT left
if (deploymentSucceeded.get() > PERSIST_LIMIT) {
childrenToRemove.stream()
.sorted(Comparator.comparingLong(t -> t.find(DEPLOYMENT_STATUS_KEY_NAME).getModtime()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find may return null, you must handle this correctly.

.limit(childrenToRemove.size() - PERSIST_LIMIT)
.forEach(Node::remove);
}
}
}

@SuppressFBWarnings("SIC_INNER_SHOULD_BE_STATIC")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ class CLIEventStreamAgentTest {

@BeforeEach
void setup() throws AuthorizationException {
when(mockContext.getContinuation()).thenReturn(mock(ServerConnectionContinuation.class));
when(mockContext.getAuthenticationData()).thenReturn(() -> String.format(GREENGRASS_CLI_CLIENT_ID_FMT, "abc"));
lenient().when(mockContext.getContinuation()).thenReturn(mock(ServerConnectionContinuation.class));
lenient().when(mockContext.getAuthenticationData()).thenReturn(() -> String.format(GREENGRASS_CLI_CLIENT_ID_FMT, "abc"));
lenient().when(authorizationHandler.isAuthorized(eq(CLI_SERVICE), any()))
.thenThrow(new AuthorizationException("bad"));
}
Expand Down Expand Up @@ -533,4 +533,31 @@ void test_createDebugPassword() throws IOException {
assertEquals("ABCDE", response.getCertificateSHA256Hash());
}
}

@Test
void test_persist_last_five_successful_local_deployments() throws InterruptedException {
Context context = new Context();
Topics cliServiceConfig = Topics.of(context, TEST_SERVICE, null);

CLIEventStreamAgent.LocalDeploymentDetails localDeploymentDetails = new CLIEventStreamAgent.LocalDeploymentDetails();
for (int i = 1; i <= 7; i++) {
String deploymentId = "deploymentId" + i;
localDeploymentDetails.setDeploymentId(deploymentId);
localDeploymentDetails.setDeploymentType(Deployment.DeploymentType.LOCAL);
if (i == 3) {
localDeploymentDetails.setStatus(DeploymentStatus.FAILED);
} else {
localDeploymentDetails.setStatus(DeploymentStatus.SUCCEEDED);
}
cliEventStreamAgent.persistLocalDeployment(cliServiceConfig, localDeploymentDetails.convertToMapOfObject());
// to ensure topic add as order
Thread.sleep(1);
context.waitForPublishQueueToClear();
}

Topics localDeployments = cliServiceConfig.findTopics(PERSISTENT_LOCAL_DEPLOYMENTS);

assertEquals(6, localDeployments.size());
assertNull(localDeployments.findNode("deploymentId1"));
}
}