From e7c7e9cc0d9a06a37bd9bc6c74b996e883de01e9 Mon Sep 17 00:00:00 2001 From: Dongbo Xiao Date: Wed, 17 Feb 2021 12:22:36 -0500 Subject: [PATCH 1/2] correctly handle large number of secrets and configmaps --- .../helpers/DomainValidationSteps.java | 23 ++- .../helpers/DomainValidationStepTest.java | 159 +++++++++++++++++- 2 files changed, 177 insertions(+), 5 deletions(-) diff --git a/operator/src/main/java/oracle/kubernetes/operator/helpers/DomainValidationSteps.java b/operator/src/main/java/oracle/kubernetes/operator/helpers/DomainValidationSteps.java index cc58fd80b7b..c4658108fc0 100644 --- a/operator/src/main/java/oracle/kubernetes/operator/helpers/DomainValidationSteps.java +++ b/operator/src/main/java/oracle/kubernetes/operator/helpers/DomainValidationSteps.java @@ -3,6 +3,7 @@ package oracle.kubernetes.operator.helpers; +import java.util.ArrayList; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -47,7 +48,7 @@ public static Step createDomainValidationSteps(String namespace, Step next) { new DomainValidationStep(next)); } - public static Step createAdditionalDomainValidationSteps(V1PodSpec podSpec) { + static Step createAdditionalDomainValidationSteps(V1PodSpec podSpec) { return new DomainAdditionalValidationStep(podSpec); } @@ -59,7 +60,7 @@ private static Step createListSecretsStep(String domainNamespace) { return new CallBuilder().listSecretsAsync(domainNamespace, new ListSecretsResponseStep()); } - public static Step createValidateDomainTopologyStep(Step next) { + static Step createValidateDomainTopologyStep(Step next) { return new ValidateDomainTopologyStep(next); } @@ -67,10 +68,17 @@ static class ListSecretsResponseStep extends DefaultResponseStep { @Override public NextAction onSuccess(Packet packet, CallResponse callResponse) { - packet.put(SECRETS, callResponse.getResult().getItems()); + List list = getSecrets(packet); + list.addAll(callResponse.getResult().getItems()); + packet.put(SECRETS, list); return doContinueListOrNext(callResponse, packet); } + + @SuppressWarnings("unchecked") + static List getSecrets(Packet packet) { + return Optional.ofNullable((List) packet.get(SECRETS)).orElse(new ArrayList<>()); + } } private static Step createListConfigMapsStep(String domainNamespace) { @@ -81,10 +89,17 @@ static class ListConfigMapsResponseStep extends DefaultResponseStep callResponse) { - packet.put(CONFIGMAPS, callResponse.getResult().getItems()); + List list = getConfigMaps(packet); + list.addAll(callResponse.getResult().getItems()); + packet.put(CONFIGMAPS, list); return doContinueListOrNext(callResponse, packet); } + + @SuppressWarnings("unchecked") + static List getConfigMaps(Packet packet) { + return Optional.ofNullable((List) packet.get(CONFIGMAPS)).orElse(new ArrayList<>()); + } } static class DomainValidationStep extends Step { diff --git a/operator/src/test/java/oracle/kubernetes/operator/helpers/DomainValidationStepTest.java b/operator/src/test/java/oracle/kubernetes/operator/helpers/DomainValidationStepTest.java index 9a656835be3..d759d062a37 100644 --- a/operator/src/test/java/oracle/kubernetes/operator/helpers/DomainValidationStepTest.java +++ b/operator/src/test/java/oracle/kubernetes/operator/helpers/DomainValidationStepTest.java @@ -4,15 +4,18 @@ package oracle.kubernetes.operator.helpers; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.logging.LogRecord; +import java.util.stream.IntStream; import com.meterware.simplestub.Memento; import com.meterware.simplestub.StaticStubSupport; import com.meterware.simplestub.Stub; +import io.kubernetes.client.openapi.models.V1ConfigMap; import io.kubernetes.client.openapi.models.V1Event; import io.kubernetes.client.openapi.models.V1ObjectMeta; import io.kubernetes.client.openapi.models.V1Secret; @@ -28,10 +31,12 @@ import oracle.kubernetes.operator.work.TerminalStep; import oracle.kubernetes.utils.TestUtils; import oracle.kubernetes.weblogic.domain.model.Cluster; +import oracle.kubernetes.weblogic.domain.model.Configuration; import oracle.kubernetes.weblogic.domain.model.ConfigurationConstants; import oracle.kubernetes.weblogic.domain.model.Domain; import oracle.kubernetes.weblogic.domain.model.DomainStatus; import oracle.kubernetes.weblogic.domain.model.ManagedServer; +import oracle.kubernetes.weblogic.domain.model.Model; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -42,6 +47,7 @@ import static oracle.kubernetes.operator.EventTestUtils.containsEventWithMessage; import static oracle.kubernetes.operator.ProcessingConstants.DOMAIN_TOPOLOGY; import static oracle.kubernetes.operator.ProcessingConstants.MAKE_RIGHT_DOMAIN_OPERATION; +import static oracle.kubernetes.operator.TuningParametersImpl.DEFAULT_CALL_LIMIT; import static oracle.kubernetes.operator.helpers.KubernetesTestSupport.DOMAIN; import static oracle.kubernetes.operator.helpers.ServiceHelperTestBase.NS; import static oracle.kubernetes.operator.logging.MessageKeys.DOMAIN_VALIDATION_FAILED; @@ -57,6 +63,17 @@ import static org.hamcrest.junit.MatcherAssert.assertThat; public class DomainValidationStepTest { + /** More than one chunk's worth of secrets or configmaps. */ + private static final int MULTI_CHUNKS_FIRST_NUM_IN_SECOND_CHUNK = DEFAULT_CALL_LIMIT + 1; + private static final int MULTI_CHUNKS_MIDDLE_NUM_IN_FIRST_CHUNK = DEFAULT_CALL_LIMIT / 2; + private static final int MULTI_CHUNKS_LAST_NUM = DEFAULT_CALL_LIMIT * 2 + 1; + + private static final String SECRETS = "secrets"; + private static final String CONFIGMAPS = "configmaps"; + + private static final String TEST_SECRET_PREFIX = "TEST_SECRET"; + private static final String TEST_CONFIGMAP_PREFIX = "TEST_CM"; + private final Domain domain = DomainProcessorTestSetup.createTestDomain(); private final DomainPresenceInfo info = new DomainPresenceInfo(domain); private final TerminalStep terminalStep = new TerminalStep(); @@ -180,6 +197,146 @@ public void whenDomainRefersToDefinedSecret_runNextStep() { assertThat(terminalStep.wasRun(), is(true)); } + @Test + public void whenDomainValidationStepsCalled_withSecretInMultiChunks_packetContainsAllSecrets() { + createSecrets(MULTI_CHUNKS_LAST_NUM); + testSupport.runSteps(domainValidationSteps); + + assertThat(getMatchingSecretsCount(), is(MULTI_CHUNKS_LAST_NUM)); + } + + private int getMatchingSecretsCount() { + List list = + (List) Optional.ofNullable(testSupport.getPacket().get(SECRETS)).orElse(Collections.EMPTY_LIST); + return Math.toIntExact(list.stream().filter(secret -> matchesExpectedSecretNamePattern(secret)).count()); + } + + private boolean matchesExpectedSecretNamePattern(V1Secret secret) { + return Optional.of(secret).map(V1Secret::getMetadata).map(V1ObjectMeta::getName).orElse("") + .startsWith(TEST_SECRET_PREFIX); + } + + @Test + public void whenDomainRefersToDefinedSecretInMiddleChunk_runNextStep() { + domain.getSpec().withWebLogicCredentialsSecret( + new V1SecretReference().name(TEST_SECRET_PREFIX + MULTI_CHUNKS_FIRST_NUM_IN_SECOND_CHUNK).namespace(NS)); + createSecrets(MULTI_CHUNKS_LAST_NUM); + testSupport.runSteps(domainValidationSteps); + + assertThat(terminalStep.wasRun(), is(true)); + } + + @Test + public void whenDomainRefersToDefinedSecretInFirstChunk_runNextStep() { + domain.getSpec().withWebLogicCredentialsSecret( + new V1SecretReference().name(TEST_SECRET_PREFIX + MULTI_CHUNKS_MIDDLE_NUM_IN_FIRST_CHUNK).namespace(NS)); + createSecrets(MULTI_CHUNKS_LAST_NUM); + testSupport.runSteps(domainValidationSteps); + + assertThat(terminalStep.wasRun(), is(true)); + } + + @Test + public void whenDomainRefersToDefinedSecretInLastChunk_runNextStep() { + domain.getSpec().withWebLogicCredentialsSecret( + new V1SecretReference().name(TEST_SECRET_PREFIX + MULTI_CHUNKS_LAST_NUM).namespace(NS)); + createSecrets(MULTI_CHUNKS_LAST_NUM); + testSupport.runSteps(domainValidationSteps); + + assertThat(terminalStep.wasRun(), is(true)); + } + + private void createSecrets(int lastSecretNum) { + IntStream.rangeClosed(1, lastSecretNum) + .boxed() + .map(i -> TEST_SECRET_PREFIX + i) + .map(this::createSecret) + .forEach(testSupport::defineResources); + } + + private V1Secret createSecret(String secret) { + return new V1Secret().metadata(new V1ObjectMeta().name(secret).namespace(NS)); + } + + @Test + public void whenDomainValidationStepsCalled_withConfigMapInMultiChunks_packetContainsAllConfigMaps() { + createConfigMaps(MULTI_CHUNKS_LAST_NUM); + testSupport.runSteps(domainValidationSteps); + + assertThat(getMatchingConfigMapsCount(), is(MULTI_CHUNKS_LAST_NUM)); + } + + private int getMatchingConfigMapsCount() { + List list = + (List) Optional.ofNullable(testSupport.getPacket().get(CONFIGMAPS)).orElse(Collections.EMPTY_LIST); + return Math.toIntExact(list.stream().filter(cm -> matchesExpectedConfigMapNamePattern(cm)).count()); + } + + private boolean matchesExpectedConfigMapNamePattern(V1ConfigMap cm) { + return Optional.of(cm).map(V1ConfigMap::getMetadata).map(V1ObjectMeta::getName).orElse("") + .startsWith(TEST_CONFIGMAP_PREFIX); + } + + @Test + public void whenDomainRefersToDefinedConfigMapInMiddleChunk_runNextStep() { + domain.getSpec() + .withWebLogicCredentialsSecret(new V1SecretReference().name("name")) + .setConfiguration(new Configuration().withModel( + new Model().withConfigMap(TEST_CONFIGMAP_PREFIX + MULTI_CHUNKS_FIRST_NUM_IN_SECOND_CHUNK) + .withRuntimeEncryptionSecret("name"))); + + testSupport.defineResources(new V1Secret().metadata(new V1ObjectMeta().name("name").namespace(NS))); + + createConfigMaps(MULTI_CHUNKS_LAST_NUM); + testSupport.runSteps(domainValidationSteps); + + assertThat(terminalStep.wasRun(), is(true)); + } + + @Test + public void whenDomainRefersToDefinedConfigMapInFirstChunk_runNextStep() { + domain.getSpec() + .withWebLogicCredentialsSecret(new V1SecretReference().name("name")) + .setConfiguration(new Configuration().withModel( + new Model().withConfigMap(TEST_CONFIGMAP_PREFIX + MULTI_CHUNKS_MIDDLE_NUM_IN_FIRST_CHUNK) + .withRuntimeEncryptionSecret("name"))); + + testSupport.defineResources(new V1Secret().metadata(new V1ObjectMeta().name("name").namespace(NS))); + + createConfigMaps(MULTI_CHUNKS_LAST_NUM); + testSupport.runSteps(domainValidationSteps); + + assertThat(terminalStep.wasRun(), is(true)); + } + + @Test + public void whenDomainRefersToDefinedConfigMapInLastChunk_runNextStep() { + domain.getSpec() + .withWebLogicCredentialsSecret(new V1SecretReference().name("name")) + .setConfiguration(new Configuration().withModel( + new Model().withConfigMap(TEST_CONFIGMAP_PREFIX + MULTI_CHUNKS_LAST_NUM) + .withRuntimeEncryptionSecret("name"))); + + testSupport.defineResources(new V1Secret().metadata(new V1ObjectMeta().name("name").namespace(NS))); + + createConfigMaps(MULTI_CHUNKS_LAST_NUM); + testSupport.runSteps(domainValidationSteps); + + assertThat(terminalStep.wasRun(), is(true)); + } + + private void createConfigMaps(int lastConfigMapNum) { + IntStream.rangeClosed(1, lastConfigMapNum) + .boxed() + .map(i -> TEST_CONFIGMAP_PREFIX + i) + .map(this::createConfigMap) + .forEach(testSupport::defineResources); + } + + private V1ConfigMap createConfigMap(String cm) { + return new V1ConfigMap().metadata(new V1ObjectMeta().name(cm).namespace(NS)); + } + @Test public void whenClusterDoesNotExistInDomain_logWarning() { domain.getSpec().withCluster(createCluster("no-such-cluster")); @@ -241,7 +398,7 @@ public void whenBothServerAndClusterDoNotExistInDomain_createEventWithBothWarnin } @Test - public void whenIsExplictRecheck_doNotCreateEvent() { + public void whenIsExplicitRecheck_doNotCreateEvent() { consoleControl.ignoreMessage(NO_CLUSTER_IN_DOMAIN); setExplicitRecheck(); domain.getSpec().withCluster(createCluster("no-such-cluster")); From 9426b2bab27d4e70a5088320d736cbf96893f46c Mon Sep 17 00:00:00 2001 From: Dongbo Xiao Date: Wed, 17 Feb 2021 14:32:50 -0500 Subject: [PATCH 2/2] start first chunk with empty list --- .../operator/helpers/DomainValidationSteps.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/operator/src/main/java/oracle/kubernetes/operator/helpers/DomainValidationSteps.java b/operator/src/main/java/oracle/kubernetes/operator/helpers/DomainValidationSteps.java index c4658108fc0..d90566f9760 100644 --- a/operator/src/main/java/oracle/kubernetes/operator/helpers/DomainValidationSteps.java +++ b/operator/src/main/java/oracle/kubernetes/operator/helpers/DomainValidationSteps.java @@ -17,6 +17,7 @@ import oracle.kubernetes.operator.DomainStatusUpdater; import oracle.kubernetes.operator.MakeRightDomainOperation; import oracle.kubernetes.operator.ProcessingConstants; +import oracle.kubernetes.operator.calls.AsyncRequestStep; import oracle.kubernetes.operator.calls.CallResponse; import oracle.kubernetes.operator.helpers.EventHelper.EventData; import oracle.kubernetes.operator.helpers.EventHelper.EventItem; @@ -75,9 +76,13 @@ public NextAction onSuccess(Packet packet, CallResponse callRespon return doContinueListOrNext(callResponse, packet); } - @SuppressWarnings("unchecked") static List getSecrets(Packet packet) { - return Optional.ofNullable((List) packet.get(SECRETS)).orElse(new ArrayList<>()); + return Optional.ofNullable(getSecretsIfContinue(packet)).orElse(new ArrayList<>()); + } + + @SuppressWarnings("unchecked") + private static List getSecretsIfContinue(Packet packet) { + return packet.get(AsyncRequestStep.CONTINUE) != null ? (List) packet.get(SECRETS) : null; } } @@ -96,9 +101,13 @@ public NextAction onSuccess(Packet packet, CallResponse callRes return doContinueListOrNext(callResponse, packet); } - @SuppressWarnings("unchecked") static List getConfigMaps(Packet packet) { - return Optional.ofNullable((List) packet.get(CONFIGMAPS)).orElse(new ArrayList<>()); + return Optional.ofNullable(getConfigMapsIfContinue(packet)).orElse(new ArrayList<>()); + } + + @SuppressWarnings("unchecked") + private static List getConfigMapsIfContinue(Packet packet) { + return packet.get(AsyncRequestStep.CONTINUE) != null ? (List) packet.get(CONFIGMAPS) : null; } }