Skip to content

fix(operator): update ConfigurationPlan in-place on spec update #102

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.

Sign up for GitHub

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

Open
Vaishnav88sk wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(operator): update ConfigurationPlan in-place on spec update #102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 35 additions & 14 deletions operator/src/main/java/io/reshapr/kubernetes/operator/ConfigurationPlanReconciler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import jakarta.inject.Inject;
import org.jboss.logging.Logger;

import java.util.Map;

import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_ALL_NAMESPACES;

/**
Expand Down Expand Up @@ -95,7 +97,8 @@ protected UpdateControl<ConfigurationPlan> doReconcile(ConfigurationPlan resourc
}

String backendSecretId = null;
String organization = resource.getMetadata().getAnnotations().get(ReshaprAnnotations.ORGANIZATION);
Map<String, String> annotations = resource.getMetadata().getAnnotations();
String organization = annotations != null ? annotations.get(ReshaprAnnotations.ORGANIZATION) : null;

if (spec.getOauth2() != null) {
OAuth2Spec oauth2 = spec.getOauth2();
Expand Down Expand Up @@ -151,21 +154,39 @@ protected UpdateControl<ConfigurationPlan> doReconcile(ConfigurationPlan resourc
remotePlan.setBackendSecretId(backendSecretId);
}

io.reshapr.client.model.ConfigurationPlan createdPlan = api.createConfigPlan(remotePlan);
String newPlanId = createdPlan.getId();

if (spec.isApiKey()) {
api.renewConfigPlanApiKey(newPlanId);
String newPlanId;
if (alreadyImported) {
remotePlan.setId(currentPlanId);
try {
io.reshapr.client.model.ConfigurationPlan updated = api.updateConfigPlan(currentPlanId, remotePlan);
newPlanId = updated != null && updated.getId() != null ? updated.getId() : currentPlanId;
logger.infof("ConfigurationPlan '%s' updated in control plane with id=%s", name, newPlanId);
} catch (ApiException e) {
if (e.getCode() == 404) {
logger.warnf("Recorded control plane ConfigurationPlan id=%s for '%s' no longer exists (HTTP 404) — recreating",
currentPlanId, name);
io.reshapr.client.model.ConfigurationPlan created = api.createConfigPlan(remotePlan);
newPlanId = created != null ? created.getId() : null;
if (spec.isApiKey() && newPlanId != null) {
api.renewConfigPlanApiKey(newPlanId);
}
} else {
throw e;
}
}
} else {
io.reshapr.client.model.ConfigurationPlan createdPlan = api.createConfigPlan(remotePlan);
newPlanId = createdPlan != null ? createdPlan.getId() : null;
logger.infof("ConfigurationPlan '%s' created in control plane with id=%s", name, newPlanId);
if (spec.isApiKey() && newPlanId != null) {
api.renewConfigPlanApiKey(newPlanId);
}
}

if (alreadyImported && !newPlanId.equals(currentPlanId)) {
try {
api.deleteConfigPlan(currentPlanId);
} catch (ApiException e) {
if (e.getCode() != 404) {
logger.warnf(e, "Failed to delete previous ConfigurationPlan id=%s", currentPlanId);
}
}
if (newPlanId == null) {
logger.warnf("ConfigurationPlan '%s': control plane did not return an id — rescheduling", name);
return recordStatus(resource, Status.IN_PROGRESS,
"Control plane did not return a ConfigurationPlan id").rescheduleAfter(RETRY_DELAY_MS);
}

status.setStatus(Status.READY);
Expand Down
84 changes: 84 additions & 0 deletions operator/src/test/java/io/reshapr/kubernetes/operator/ConfigurationPlanReconcilerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import io.reshapr.kubernetes.api.configurationplan.v1alpha1.CachePolicy;
import io.reshapr.kubernetes.api.configurationplan.v1alpha1.ConfigurationPlan;
import io.reshapr.kubernetes.api.configurationplan.v1alpha1.ConfigurationPlanSpec;
import io.reshapr.kubernetes.api.configurationplan.v1alpha1.ConfigurationPlanStatus;
import io.reshapr.kubernetes.api.configurationplan.v1alpha1.HeaderPolicy;
import io.reshapr.kubernetes.api.configurationplan.v1alpha1.HeaderRename;
import io.reshapr.kubernetes.api.configurationplan.v1alpha1.HeaderRules;
Expand Down Expand Up @@ -277,4 +278,87 @@ void doReconcile_withMinimalSpec_usesDefaultToolExposureMode() throws Exception
assertTrue(sentPlan.getExcludedOperations() == null || sentPlan.getExcludedOperations().isEmpty());
}
}

@Test
void doReconcile_whenAlreadyImportedAndSpecChanged_updatesInPlaceInsteadOfRecreating() throws Exception {
ReshaprApiClientFactory factory = mock(ReshaprApiClientFactory.class);
ConfigurationPlanReconciler reconciler = new ConfigurationPlanReconciler(factory);
ApiClient apiClient = mock(ApiClient.class);
ConfigurationPlan cp = buildFullSpecConfigurationPlan();

ConfigurationPlanStatus status = new ConfigurationPlanStatus();
status.setConfigurationPlanId(REMOTE_PLAN_ID);
status.setObservedGeneration(1L);
status.setStatus(io.reshapr.kubernetes.api.model.Status.READY);
cp.setStatus(status);
cp.getMetadata().setGeneration(2L);

try (MockedConstruction<DefaultApi> mocked = Mockito.mockConstruction(DefaultApi.class,
(mock, ctx) -> {
when(mock.getServices(any(BigDecimal.class), any(BigDecimal.class)))
.thenReturn(List.of(remoteService()));

Secret createdSecret = new Secret();
createdSecret.setId(REMOTE_SECRET_ID);
when(mock.createSecret(any(Secret.class))).thenReturn(createdSecret);

io.reshapr.client.model.ConfigurationPlan updated = new io.reshapr.client.model.ConfigurationPlan();
updated.setId(REMOTE_PLAN_ID);
when(mock.updateConfigPlan(Mockito.eq(REMOTE_PLAN_ID), any(io.reshapr.client.model.ConfigurationPlan.class)))
.thenReturn(updated);
})) {

reconciler.doReconcile(cp, null, apiClient);

DefaultApi mockedApi = mocked.constructed().get(0);
verify(mockedApi).updateConfigPlan(Mockito.eq(REMOTE_PLAN_ID), any(io.reshapr.client.model.ConfigurationPlan.class));
verify(mockedApi, never()).createConfigPlan(any());
verify(mockedApi, never()).deleteConfigPlan(any());
}
}

@Test
void doReconcile_whenAlreadyImportedAndNotFound_recreatesConfigPlan() throws Exception {
ReshaprApiClientFactory factory = mock(ReshaprApiClientFactory.class);
ConfigurationPlanReconciler reconciler = new ConfigurationPlanReconciler(factory);
ApiClient apiClient = mock(ApiClient.class);
ConfigurationPlan cp = buildFullSpecConfigurationPlan();

ConfigurationPlanStatus status = new ConfigurationPlanStatus();
status.setConfigurationPlanId("stale-plan-id");
status.setObservedGeneration(1L);
status.setStatus(io.reshapr.kubernetes.api.model.Status.READY);
cp.setStatus(status);
cp.getMetadata().setGeneration(2L);

try (MockedConstruction<DefaultApi> mocked = Mockito.mockConstruction(DefaultApi.class,
(mock, ctx) -> {
when(mock.getServices(any(BigDecimal.class), any(BigDecimal.class)))
.thenReturn(List.of(remoteService()));

Secret createdSecret = new Secret();
createdSecret.setId(REMOTE_SECRET_ID);
when(mock.createSecret(any(Secret.class))).thenReturn(createdSecret);

// 404 on update
when(mock.updateConfigPlan(Mockito.eq("stale-plan-id"), any(io.reshapr.client.model.ConfigurationPlan.class)))
.thenThrow(new io.reshapr.client.ApiException(404, "Not Found"));

io.reshapr.client.model.ConfigurationPlan recreated = new io.reshapr.client.model.ConfigurationPlan();
recreated.setId(REMOTE_PLAN_ID);
when(mock.createConfigPlan(any(io.reshapr.client.model.ConfigurationPlan.class)))
.thenReturn(recreated);
})) {

reconciler.doReconcile(cp, null, apiClient);

DefaultApi mockedApi = mocked.constructed().get(0);
verify(mockedApi).updateConfigPlan(Mockito.eq("stale-plan-id"), any(io.reshapr.client.model.ConfigurationPlan.class));
verify(mockedApi).createConfigPlan(any(io.reshapr.client.model.ConfigurationPlan.class));
verify(mockedApi).renewConfigPlanApiKey(REMOTE_PLAN_ID);
assertEquals(REMOTE_PLAN_ID, cp.getStatus().getConfigurationPlanId());
}
}
}


Loading