Skip to content

Commit

Permalink
Skip validating domain list ability if operator has already read, cre…
Browse files Browse the repository at this point in the history 
…ated, or updated CRD
  • Loading branch information
@rjeberhard
rjeberhard committed Feb 16, 2022
1 parent b0a64e6 commit 4586bc1
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 29 deletions.
31 changes: 29 additions & 2 deletions operator/src/main/java/oracle/kubernetes/operator/Main.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import javax.annotation.Nonnull;

import io.kubernetes.client.openapi.models.CoreV1EventList;
import io.kubernetes.client.openapi.models.V1CustomResourceDefinition;
import io.kubernetes.client.openapi.models.V1Namespace;
import io.kubernetes.client.openapi.models.V1NamespaceList;
import io.kubernetes.client.openapi.models.V1ObjectMeta;
Expand Down Expand Up @@ -137,6 +138,7 @@ static class MainDelegateImpl implements MainDelegate, DomainProcessorDelegate {
private final Engine engine;
private final DomainProcessor domainProcessor;
private final DomainNamespaces domainNamespaces;
private final AtomicReference<V1CustomResourceDefinition> crdRefernce;

public MainDelegateImpl(Properties buildProps, ScheduledExecutorService scheduledExecutorService) {
buildVersion = getBuildVersion(buildProps);
Expand All @@ -152,6 +154,8 @@ public MainDelegateImpl(Properties buildProps, ScheduledExecutorService schedule
domainNamespaces = new DomainNamespaces(productVersion);

PodHelper.setProductVersion(productVersion.toString());

crdRefernce = new AtomicReference<>();
}

private static String getBuildVersion(Properties buildProps) {
Expand Down Expand Up @@ -248,6 +252,11 @@ public FiberGate createFiberGate() {
public ScheduledFuture<?> scheduleWithFixedDelay(Runnable command, long initialDelay, long delay, TimeUnit unit) {
return engine.getExecutor().scheduleWithFixedDelay(command, initialDelay, delay, unit);
}

@Override
public AtomicReference<V1CustomResourceDefinition> getCrdReference() {
return crdRefernce;
}
}

/**
Expand Down Expand Up @@ -428,7 +437,7 @@ private Step createDomainRecheckSteps(OffsetDateTime now) {
final DomainRecheck domainRecheck = new DomainRecheck(delegate, isFullRecheck);
return Step.chain(
domainRecheck.createOperatorNamespaceReview(),
CrdHelper.createDomainCrdStep(delegate.getKubernetesVersion(), delegate.getProductVersion()),
CrdHelper.createDomainCrdStep(delegate),
createCRDPresenceCheck(),
domainRecheck.createReadNamespacesStep());
}
Expand All @@ -437,12 +446,30 @@ private Step createDomainRecheckSteps(OffsetDateTime now) {
// domains in the operator's namespace. That should succeed (although usually returning an empty list)
// if the CRD is present.
Step createCRDPresenceCheck() {
return new CallBuilder().listDomainAsync(getOperatorNamespace(), new CrdPresenceResponseStep());
return new CrdPresenceStep();
}

class CrdPresenceStep extends Step {

@Override
public NextAction apply(Packet packet) {
V1CustomResourceDefinition existingCrd = delegate.getCrdReference().get();
if (existingCrd != null) {
warnedOfCrdAbsence = false;
return doNext(packet);
}
return doNext(new CallBuilder().listDomainAsync(getOperatorNamespace(),
new CrdPresenceResponseStep(getNext())), packet);
}
}

// on failure, aborts the processing.
class CrdPresenceResponseStep extends DefaultResponseStep<DomainList> {

CrdPresenceResponseStep(Step next) {
super(next);
}

@Override
public NextAction onSuccess(Packet packet, CallResponse<DomainList> callResponse) {
warnedOfCrdAbsence = false;
Expand Down
8 changes: 6 additions & 2 deletions operator/src/main/java/oracle/kubernetes/operator/MainDelegate.java
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
// Copyright (c) 2020, 2021, Oracle and/or its affiliates.
// Copyright (c) 2020, 2022, Oracle and/or its affiliates.
// Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.

package oracle.kubernetes.operator;

import java.util.concurrent.ScheduledFuture;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicReference;

import io.kubernetes.client.openapi.models.V1CustomResourceDefinition;
import oracle.kubernetes.operator.helpers.KubernetesVersion;
import oracle.kubernetes.operator.helpers.SemanticVersion;
import oracle.kubernetes.operator.work.Packet;
Expand All @@ -14,7 +16,7 @@
/**
* Definition of an interface that returns values that the Main class requires.
*/
interface MainDelegate {
public interface MainDelegate {

SemanticVersion getProductVersion();

Expand All @@ -33,4 +35,6 @@ default void runSteps(Step firstStep) {
KubernetesVersion getKubernetesVersion();

ScheduledFuture<?> scheduleWithFixedDelay(Runnable command, long initialDelay, long delay, TimeUnit unit);

AtomicReference<V1CustomResourceDefinition> getCrdReference();
}
38 changes: 23 additions & 15 deletions operator/src/main/java/oracle/kubernetes/operator/helpers/CrdHelper.java
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2018, 2021, Oracle and/or its affiliates.
// Copyright (c) 2018, 2022, Oracle and/or its affiliates.
// Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.

package oracle.kubernetes.operator.helpers;
Expand All @@ -17,6 +17,7 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
import java.util.stream.Collectors;

Expand All @@ -35,6 +36,7 @@
import okhttp3.internal.http2.StreamResetException;
import oracle.kubernetes.operator.KubernetesConstants;
import oracle.kubernetes.operator.LabelConstants;
import oracle.kubernetes.operator.MainDelegate;
import oracle.kubernetes.operator.calls.CallResponse;
import oracle.kubernetes.operator.logging.LoggingFacade;
import oracle.kubernetes.operator.logging.LoggingFactory;
Expand Down Expand Up @@ -74,7 +76,7 @@ public static void main(String[] args) throws URISyntaxException {
}

static void writeCrdFiles(String crdFileName) throws URISyntaxException {
CrdContext context = new CrdContext(null, null, null);
CrdContext context = new CrdContext(null, null);

final URI outputFile = asFileURI(crdFileName);

Expand Down Expand Up @@ -120,8 +122,8 @@ private static void dumpYaml(Writer writer, Object model) {
// Map = gson.fromJson(Map.class)
// yaml dump ? // ordering and format likely to change massively

public static Step createDomainCrdStep(KubernetesVersion version, SemanticVersion productVersion) {
return new CrdStep(version, productVersion);
public static Step createDomainCrdStep(MainDelegate mainDelegate) {
return new CrdStep(mainDelegate);
}

private static List<ResourceVersion> getVersions(V1CustomResourceDefinition crd) {
Expand All @@ -144,8 +146,8 @@ boolean isOutdatedCrd(
static class CrdStep extends Step {
final CrdContext context;

CrdStep(KubernetesVersion version, SemanticVersion productVersion) {
context = new CrdContext(version, productVersion, this);
CrdStep(MainDelegate mainDelegate) {
context = new CrdContext(mainDelegate, this);
}

@Override
Expand All @@ -158,14 +160,12 @@ public NextAction apply(Packet packet) {
static class CrdContext {
private final Step conflictStep;
private final V1CustomResourceDefinition model;
private final KubernetesVersion version;
private final SemanticVersion productVersion;
private final MainDelegate mainDelegate;

CrdContext(KubernetesVersion version, SemanticVersion productVersion, Step conflictStep) {
this.version = version;
this.productVersion = productVersion;
CrdContext(MainDelegate mainDelegate, Step conflictStep) {
this.mainDelegate = mainDelegate;
this.conflictStep = conflictStep;
this.model = createModel(productVersion);
this.model = createModel(Optional.ofNullable(mainDelegate).map(MainDelegate::getProductVersion).orElse(null));
}

static V1CustomResourceDefinition createModel(SemanticVersion productVersion) {
Expand Down Expand Up @@ -294,7 +294,7 @@ ResponseStep<V1CustomResourceDefinition> createCreateResponseStep(Step next) {
}

private boolean isOutdatedCrd(V1CustomResourceDefinition existingCrd) {
return COMPARATOR.isOutdatedCrd(productVersion, existingCrd, this.model);
return COMPARATOR.isOutdatedCrd(mainDelegate.getProductVersion(), existingCrd, this.model);
}

private boolean existingCrdContainsVersion(V1CustomResourceDefinition existingCrd) {
Expand Down Expand Up @@ -349,6 +349,8 @@ class ReadResponseStep extends DefaultResponseStep<V1CustomResourceDefinition> {
public NextAction onSuccess(
Packet packet, CallResponse<V1CustomResourceDefinition> callResponse) {
V1CustomResourceDefinition existingCrd = callResponse.getResult();
mainDelegate.getCrdReference().set(existingCrd);

if (existingCrd == null) {
return doNext(createCrd(getNext()), packet);
} else if (isOutdatedCrd(existingCrd)) {
Expand All @@ -362,6 +364,7 @@ public NextAction onSuccess(

@Override
protected NextAction onFailureNoRetry(Packet packet, CallResponse<V1CustomResourceDefinition> callResponse) {
mainDelegate.getCrdReference().set(null);
return isNotAuthorizedOrForbidden(callResponse)
? doNext(packet) : super.onFailureNoRetry(packet, callResponse);
}
Expand All @@ -381,7 +384,10 @@ public NextAction onFailure(
@Override
public NextAction onSuccess(
Packet packet, CallResponse<V1CustomResourceDefinition> callResponse) {
LOGGER.info(MessageKeys.CREATING_CRD, callResponse.getResult().getMetadata().getName());
V1CustomResourceDefinition existingCrd = callResponse.getResult();
mainDelegate.getCrdReference().set(existingCrd);

LOGGER.info(MessageKeys.CREATING_CRD, existingCrd.getMetadata().getName());
return doNext(packet);
}

Expand All @@ -407,7 +413,9 @@ public NextAction onFailure(
@Override
public NextAction onSuccess(
Packet packet, CallResponse<V1CustomResourceDefinition> callResponse) {
LOGGER.info(MessageKeys.CREATING_CRD, callResponse.getResult().getMetadata().getName());
V1CustomResourceDefinition existingCrd = callResponse.getResult();
mainDelegate.getCrdReference().set(existingCrd);
LOGGER.info(MessageKeys.CREATING_CRD, existingCrd.getMetadata().getName());
return doNext(packet);
}

Expand Down
28 changes: 27 additions & 1 deletion operator/src/test/java/oracle/kubernetes/operator/MainTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) 2018, 2021, Oracle and/or its affiliates.
// Copyright (c) 2018, 2022, Oracle and/or its affiliates.
// Licensed under the Universal Permissive License v 1.0 as shown at https://oss.oracle.com/licenses/upl.

package oracle.kubernetes.operator;
Expand All @@ -15,6 +15,7 @@
import java.util.Properties;
import java.util.Random;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicReference;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.stream.Collectors;
Expand All @@ -24,6 +25,7 @@
import com.meterware.simplestub.StaticStubSupport;
import io.kubernetes.client.openapi.models.CoreV1Event;
import io.kubernetes.client.openapi.models.V1ConfigMap;
import io.kubernetes.client.openapi.models.V1CustomResourceDefinition;
import io.kubernetes.client.openapi.models.V1Namespace;
import io.kubernetes.client.openapi.models.V1ObjectMeta;
import io.kubernetes.client.openapi.models.V1ObjectReference;
Expand Down Expand Up @@ -358,12 +360,23 @@ private void runCreateReadNamespacesStep() {
void whenNoCRD_logReasonForFailure() {
loggerControl.withLogLevel(Level.SEVERE).collectLogMessages(logRecords, CRD_NOT_INSTALLED);
simulateMissingCRD();
delegate.hideCRD();

recheckDomains();

assertThat(logRecords, containsSevere(CRD_NOT_INSTALLED));
}

@Test
void whenCRDCreated_dontLogFailure() {
loggerControl.withLogLevel(Level.SEVERE).collectLogMessages(logRecords, CRD_NOT_INSTALLED);
simulateMissingCRD();

recheckDomains();

assertThat(logRecords, not(containsSevere(CRD_NOT_INSTALLED)));
}

@Test
void afterLoggedCRDMissing_dontDoItASecondTime() {
loggerControl.withLogLevel(Level.SEVERE).collectLogMessages(logRecords, CRD_NOT_INSTALLED);
Expand All @@ -380,6 +393,7 @@ void afterLoggedCRDMissing_dontDoItASecondTime() {
void afterMissingCRDdetected_correctionOfTheConditionAllowsProcessingToOccur() {
defineSelectionStrategy(SelectionStrategy.Dedicated);
simulateMissingCRD();
delegate.hideCRD();
recheckDomains();

testSupport.cancelFailures();
Expand All @@ -397,6 +411,7 @@ void afterMissingCRDcorrected_subsequentFailureLogsReasonForFailure() {

loggerControl.withLogLevel(Level.SEVERE).collectLogMessages(logRecords, CRD_NOT_INSTALLED);
simulateMissingCRD();
delegate.hideCRD();
recheckDomains();

assertThat(logRecords, containsSevere(CRD_NOT_INSTALLED));
Expand Down Expand Up @@ -1155,6 +1170,8 @@ void withNamespaceDedicated_changeToList_onCreateReadNamespaces_StartManagingNSE
abstract static class MainDelegateStub implements MainDelegate {
private final FiberTestSupport testSupport;
private final DomainNamespaces domainNamespaces;
private final AtomicReference<V1CustomResourceDefinition> crdReference = new AtomicReference<>();
private boolean hideCRD = false;

public MainDelegateStub(FiberTestSupport testSupport, DomainNamespaces domainNamespaces) {
this.testSupport = testSupport;
Expand Down Expand Up @@ -1187,6 +1204,15 @@ public KubernetesVersion getKubernetesVersion() {
public SemanticVersion getProductVersion() {
return SemanticVersion.TEST_VERSION;
}

public void hideCRD() {
this.hideCRD = true;
}

@Override
public AtomicReference<V1CustomResourceDefinition> getCrdReference() {
return hideCRD ? new AtomicReference<>() : crdReference;
}
}

static class TestStepFactory implements Main.NextStepFactory {
Expand Down
Loading

0 comments on commit 4586bc1

Please sign in to comment.