Skip to content

Commit eff9fb8

Browse files
chunshi-orachunshi-ora
committed
Updated so that a service will be re-created if label, annotation and/or selector is changed. Adding testcases (WIP).
1 parent 806287e commit eff9fb8

File tree

2 files changed

+159
-24
lines changed

2 files changed

+159
-24
lines changed

‎operator/src/main/java/oracle/kubernetes/operator/helpers/ServiceHelper.java

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -213,12 +213,12 @@ ServerSpec getServerSpec() {
213213

214214
@Override
215215
protected Map<String, String> getServiceLabels() {
216-
return getServerSpec().getPodLabels();
216+
return getServerSpec().getServiceLabels();
217217
}
218218

219219
@Override
220220
protected Map<String, String> getServiceAnnotations() {
221-
return getServerSpec().getPodAnnotations();
221+
return getServerSpec().getServiceAnnotations();
222222
}
223223

224224
String getServerName() {
@@ -729,23 +729,34 @@ Map<String, String> getServiceAnnotations() {
729729
}
730730

731731
private static boolean validateCurrentService(V1Service build, V1Service current) {
732-
V1ServiceSpec buildSpec = build.getSpec();
733-
V1ServiceSpec currentSpec = current.getSpec();
732+
return isCurrentServiceMetadataValid(build.getMetadata(), current.getMetadata())
733+
&& isCurrentServiceSpecValid(build.getSpec(), current.getSpec());
734+
}
734735

735-
if (!VersionHelper.matchesResourceVersion(
736-
current.getMetadata(), VersionConstants.DEFAULT_DOMAIN_VERSION)) {
737-
return false;
738-
}
736+
private static boolean isCurrentServiceMetadataValid(
737+
V1ObjectMeta buildMeta, V1ObjectMeta currentMeta) {
738+
return VersionHelper.matchesResourceVersion(
739+
currentMeta, VersionConstants.DEFAULT_DOMAIN_VERSION)
740+
&& KubernetesUtils.areLabelsValid(buildMeta, currentMeta)
741+
&& KubernetesUtils.areAnnotationsValid(buildMeta, currentMeta);
742+
}
739743

744+
private static boolean isCurrentServiceSpecValid(
745+
V1ServiceSpec buildSpec, V1ServiceSpec currentSpec) {
740746
String buildType = buildSpec.getType();
741747
String currentType = currentSpec.getType();
748+
742749
if (currentType == null) {
743750
currentType = "ClusterIP";
744751
}
745752
if (!currentType.equals(buildType)) {
746753
return false;
747754
}
748755

756+
if (!KubernetesUtils.mapEquals(buildSpec.getSelector(), currentSpec.getSelector())) {
757+
return false;
758+
}
759+
749760
List<V1ServicePort> buildPorts = buildSpec.getPorts();
750761
List<V1ServicePort> currentPorts = currentSpec.getPorts();
751762

‎operator/src/test/java/oracle/kubernetes/operator/helpers/ServiceHelperTest.java

Lines changed: 140 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
import static oracle.kubernetes.operator.logging.MessageKeys.MANAGED_SERVICE_EXISTS;
3030
import static oracle.kubernetes.operator.logging.MessageKeys.MANAGED_SERVICE_REPLACED;
3131
import static org.hamcrest.Matchers.hasEntry;
32+
import static org.hamcrest.Matchers.hasKey;
3233
import static org.hamcrest.Matchers.is;
34+
import static org.hamcrest.Matchers.not;
3335
import static org.hamcrest.Matchers.nullValue;
3436
import static org.hamcrest.junit.MatcherAssert.assertThat;
3537

@@ -54,11 +56,9 @@
5456
import oracle.kubernetes.operator.wlsconfig.WlsDomainConfig;
5557
import oracle.kubernetes.operator.work.Step;
5658
import oracle.kubernetes.operator.work.TerminalStep;
57-
import oracle.kubernetes.weblogic.domain.AdminServerConfigurator;
58-
import oracle.kubernetes.weblogic.domain.DomainConfigurator;
59-
import oracle.kubernetes.weblogic.domain.DomainConfiguratorFactory;
6059
import oracle.kubernetes.weblogic.domain.v2.Domain;
6160
import oracle.kubernetes.weblogic.domain.v2.DomainSpec;
61+
import oracle.kubernetes.weblogic.domain.v2.ManagedServer;
6262
import org.junit.After;
6363
import org.junit.Before;
6464
import org.junit.Test;
@@ -68,10 +68,12 @@ public class ServiceHelperTest {
6868

6969
private static final String NS = "namespace";
7070
private static final String TEST_CLUSTER = "cluster-1";
71+
private static final int TEST_NODE_PORT = 7002;
7172
private static final int TEST_PORT = 7000;
7273
private static final int BAD_PORT = 9999;
7374
private static final String DOMAIN_NAME = "domain1";
7475
private static final String TEST_SERVER_NAME = "server1";
76+
private static final String TEST_ADMIN_NAME = "admin";
7577
private static final String SERVICE_NAME = "service1";
7678
private static final String UID = "uid1";
7779
private static final String BAD_VERSION = "bad-version";
@@ -463,12 +465,7 @@ public void onServerStepRunWithNoService_retryOnFailure() {
463465

464466
@Test
465467
public void onServerStepRunWithMatchingService_addToSko() {
466-
V1Service service =
467-
new V1Service()
468-
.spec(createServerServiceSpec())
469-
.metadata(
470-
new V1ObjectMeta().putLabelsItem(RESOURCE_VERSION_LABEL, DEFAULT_DOMAIN_VERSION));
471-
initializeServiceFromRecord(service);
468+
initializeServiceFromRecord(createServerService());
472469

473470
testSupport.runSteps(ServiceHelper.createForServerStep(terminalStep));
474471

@@ -480,6 +477,50 @@ public void onServerStepRunWithServiceWithBadVersion_replaceIt() {
480477
verifyServerServiceReplaced(this::withBadVersion);
481478
}
482479

480+
@Test
481+
public void onServerStepRunWithServiceWithLabelAdded_replaceIt() {
482+
createManagedServerWithLabel("anyLabel", "anyValue");
483+
verifyServerServiceReplaced(createServerService(), withLabel(createServerService()));
484+
}
485+
486+
@Test
487+
public void onServerStepRunWithServiceWithLabelValueChanged_replaceIt() {
488+
final String newLabelValue = "newValue";
489+
createManagedServerWithLabel("anyLabel", newLabelValue);
490+
verifyServerServiceReplaced(
491+
withLabel(createServerService()), withLabel(createServerService(), newLabelValue));
492+
}
493+
494+
@Test
495+
public void onServerStepRunWithServiceWithLabelRemoved_replaceIt() {
496+
verifyServerServiceReplaced(this::withLabel);
497+
}
498+
499+
@Test
500+
public void onServerStepRunWithServiceWithAnnotationAdded_replaceIt() {
501+
createManagedServerWithAnnotation("anyAnnotation", "anyValue");
502+
verifyServerServiceReplaced(createServerService(), withAnnotation(createServerService()));
503+
}
504+
505+
@Test
506+
public void onServerStepRunWithServiceWithAnnotationValueChanged_replaceIt() {
507+
final String newAnnotationValue = "newValue";
508+
createManagedServerWithAnnotation("anyAnnotation", newAnnotationValue);
509+
verifyServerServiceReplaced(
510+
withAnnotation(createServerService()),
511+
withAnnotation(createServerService(), newAnnotationValue));
512+
}
513+
514+
@Test
515+
public void onServerStepRunWithServiceWithAnnotationRemoved_replaceIt() {
516+
verifyServerServiceReplaced(this::withAnnotation);
517+
}
518+
519+
@Test
520+
public void onExternalStepRunWithServiceWithBadVersion_replaceIt() {
521+
verifyAdminServiceReplaced(this::withBadVersion);
522+
}
523+
483524
private void verifyServerServiceReplaced(V1Service oldService, V1Service newService) {
484525
initializeServiceFromRecord(oldService);
485526
expectDeleteServiceSuccessful(getServerServiceName());
@@ -490,18 +531,32 @@ private void verifyServerServiceReplaced(V1Service oldService, V1Service newServ
490531
assertThat(logRecords, containsInfo(MANAGED_SERVICE_REPLACED));
491532
}
492533

534+
private void verifyAdminServiceReplaced(ServiceMutator mutator) {
535+
verifyAdminServiceReplaced(mutator.change(createAdminService()), createAdminService());
536+
}
537+
538+
private void verifyAdminServiceReplaced(V1Service oldService, V1Service newService) {
539+
initializeAdminServiceFromRecord(oldService);
540+
expectDeleteServiceSuccessful(getAdminServiceName());
541+
expectSuccessfulCreateService(newService);
542+
543+
testSupport.runSteps(ServiceHelper.createForExternalServiceStep(terminalStep));
544+
545+
assertThat(logRecords, containsInfo(ADMIN_SERVICE_REPLACED));
546+
}
547+
493548
private void verifyServerServiceReplaced(ServiceMutator mutator) {
494549
verifyServerServiceReplaced(mutator.change(createServerService()), createServerService());
495550
}
496551

497552
private V1ServiceSpec createServerServiceSpec() {
498-
return createUntypedServerServiceSpec().type("ClusterIP").clusterIP("None");
553+
return createUntypedServerServiceSpec(TEST_SERVER_NAME).type("ClusterIP").clusterIP("None");
499554
}
500555

501-
private V1ServiceSpec createUntypedServerServiceSpec() {
556+
private V1ServiceSpec createUntypedServerServiceSpec(String serverName) {
502557
return new V1ServiceSpec()
503558
.putSelectorItem(DOMAINUID_LABEL, UID)
504-
.putSelectorItem(SERVERNAME_LABEL, TEST_SERVER_NAME)
559+
.putSelectorItem(SERVERNAME_LABEL, serverName)
505560
.putSelectorItem(CREATEDBYOPERATOR_LABEL, "true")
506561
.ports(
507562
Collections.singletonList(
@@ -512,10 +567,18 @@ private void initializeServiceFromRecord(V1Service service) {
512567
domainPresenceInfo.getServers().put(TEST_SERVER_NAME, createSko(service));
513568
}
514569

570+
private void initializeAdminServiceFromRecord(V1Service service) {
571+
domainPresenceInfo.getServers().put(TEST_ADMIN_NAME, createSko(service));
572+
}
573+
515574
private String getServerServiceName() {
516575
return LegalNames.toServerServiceName(UID, TEST_SERVER_NAME);
517576
}
518577

578+
private String getAdminServiceName() {
579+
return LegalNames.toExternalServiceName(UID, TEST_ADMIN_NAME);
580+
}
581+
519582
private void expectSuccessfulCreateService(V1Service service) {
520583
expectCreateService(service).returning(service);
521584
}
@@ -547,12 +610,31 @@ private V1Service createServerService(V1ServiceSpec serviceSpec) {
547610
.putLabelsItem(CREATEDBYOPERATOR_LABEL, "true"));
548611
}
549612

550-
private AdminServerConfigurator configureAdminServer() {
551-
return configureDomain().configureAdminServer();
613+
private V1Service createAdminService() {
614+
return createAdminService(createAdminServiceSpec());
552615
}
553616

554-
private DomainConfigurator configureDomain() {
555-
return DomainConfiguratorFactory.forDomain(domainPresenceInfo.getDomain());
617+
private V1Service createAdminService(V1ServiceSpec serviceSpec) {
618+
return new V1Service()
619+
.spec(serviceSpec)
620+
.metadata(
621+
new V1ObjectMeta()
622+
.name(getAdminServiceName())
623+
.namespace(NS)
624+
.putLabelsItem(RESOURCE_VERSION_LABEL, VersionConstants.DOMAIN_V2)
625+
.putLabelsItem(DOMAINUID_LABEL, UID)
626+
.putLabelsItem(DOMAINNAME_LABEL, DOMAIN_NAME)
627+
.putLabelsItem(SERVERNAME_LABEL, TEST_ADMIN_NAME)
628+
.putLabelsItem(CREATEDBYOPERATOR_LABEL, "true"));
629+
}
630+
631+
private V1ServiceSpec createAdminServiceSpec() {
632+
final V1ServiceSpec serviceSpec =
633+
createUntypedServerServiceSpec(TEST_ADMIN_NAME).type("NodePort");
634+
635+
serviceSpec.getPorts().stream().findAny().ifPresent(port -> port.setNodePort(TEST_NODE_PORT));
636+
637+
return serviceSpec;
556638
}
557639

558640
interface ServiceMutator {
@@ -564,6 +646,48 @@ private V1Service withBadVersion(V1Service service) {
564646
return service;
565647
}
566648

649+
private V1Service withLabel(V1Service service) {
650+
return withLabel(service, "anyValue");
651+
}
652+
653+
private V1Service withLabel(V1Service service, String labelValue) {
654+
final String labelName = "anyLabel";
655+
656+
assertThat(service.getMetadata().getLabels(), not(hasKey(labelName)));
657+
service.getMetadata().putLabelsItem(labelName, labelValue);
658+
assertThat(service.getMetadata().getLabels(), hasKey(labelName));
659+
660+
return service;
661+
}
662+
663+
private V1Service withAnnotation(V1Service service) {
664+
return withAnnotation(service, "anyValue");
665+
}
666+
667+
private V1Service withAnnotation(V1Service service, String value) {
668+
final String annotationName = "anyAnnotation";
669+
670+
assertThat(service.getMetadata().getAnnotations(), not(hasKey(annotationName)));
671+
service.getMetadata().putAnnotationsItem(annotationName, value);
672+
assertThat(service.getMetadata().getAnnotations(), hasKey(annotationName));
673+
674+
return service;
675+
}
676+
677+
private void createManagedServerWithLabel(String label, String value) {
678+
final ManagedServer ms = new ManagedServer();
679+
ms.setServerName(TEST_SERVER_NAME);
680+
ms.getServiceLabels().put(label, value);
681+
domainPresenceInfo.getDomain().getSpec().getManagedServers().add(ms);
682+
}
683+
684+
private void createManagedServerWithAnnotation(String annotation, String value) {
685+
final ManagedServer ms = new ManagedServer();
686+
ms.setServerName(TEST_SERVER_NAME);
687+
ms.getServiceAnnotations().put(annotation, value);
688+
domainPresenceInfo.getDomain().getSpec().getManagedServers().add(ms);
689+
}
690+
567691
private CallTestSupport.CannedResponse expectReadService(String serviceName) {
568692
return testSupport.createCannedResponse("readService").withNamespace(NS).withName(serviceName);
569693
}

0 commit comments

Comments
 (0)