Skip to content

Commit 04a1b93

Browse files
ppkarwaszppkarwasz
committed
Reach 80% test coverage
1 parent 3a57b2b commit 04a1b93

File tree

5 files changed

+118
-36
lines changed

5 files changed

+118
-36
lines changed

‎log4j/src/main/java/io/fabric8/kubernetes/log4j/lookup/ClientBuilder.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,9 @@ static Config kubernetesClientConfig(final PropertiesUtil props) {
7676
.build();
7777
}
7878
return base;
79-
} catch (final Throwable t) {
79+
} catch (final Exception e) {
8080
StatusLogger.getLogger().warn("An error occurred while retrieving Kubernetes Client configuration: {}.",
81-
t.getMessage(), t);
81+
e.getMessage(), e);
8282
}
8383
return null;
8484
}

‎log4j/src/main/java/io/fabric8/kubernetes/log4j/lookup/ContainerUtil.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ final class ContainerUtil {
3535

3636
private static final Logger LOGGER = StatusLogger.getLogger();
3737
private static final Pattern DOCKER_ID_PATTERN = Pattern.compile("[0-9a-fA-F]{64}");
38-
static final Path CGROUP_FILE = Paths.get("/proc/self/cgroup");
38+
static final Path CGROUP_PATH = Paths.get("/proc/self/cgroup");
3939

4040
private ContainerUtil() {
4141
}

‎log4j/src/main/java/io/fabric8/kubernetes/log4j/lookup/KubernetesLookup.java‎

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.net.InetAddress;
3636
import java.net.URL;
3737
import java.net.UnknownHostException;
38+
import java.nio.file.Path;
3839
import java.util.List;
3940
import java.util.Map;
4041
import java.util.concurrent.locks.Lock;
@@ -156,6 +157,8 @@ public class KubernetesLookup extends AbstractLookup {
156157
private static final String POD_NAME = "podName";
157158

158159
private static KubernetesInfo kubernetesInfo;
160+
// Used in tests
161+
static Path cgroupPath = ContainerUtil.CGROUP_PATH;
159162
private static final ReadWriteLock LOCK = new ReentrantReadWriteLock();
160163
private static final Lock READ_LOCK = LOCK.readLock();
161164
private static final Lock WRITE_LOCK = LOCK.writeLock();
@@ -191,7 +194,9 @@ private static void initialize(KubernetesLookup lookup) {
191194
try {
192195
kubernetesInfo = KubernetesLookup.kubernetesInfo;
193196
if (kubernetesInfo == null || isSpringStatusChanged(kubernetesInfo)) {
194-
tryInitializeFields(lookup);
197+
if (lookup.pod == null || lookup.namespace == null || lookup.masterUrl == null) {
198+
tryInitializeFields(lookup);
199+
}
195200
// Retrieve the data from the fields
196201
kubernetesInfo = new KubernetesInfo();
197202
kubernetesInfo.isSpringActive = isSpringActive();
@@ -260,8 +265,8 @@ private static Pod getCurrentPod(final KubernetesClient kubernetesClient) {
260265
if (hostName != null && !hostName.isEmpty()) {
261266
return kubernetesClient.pods().withName(hostName).get();
262267
}
263-
} catch (Throwable t) {
264-
LOGGER.debug("Unable to locate pod with name {}.", hostName);
268+
} catch (Exception e) {
269+
LOGGER.debug("Unable to locate pod with name {}.", hostName, e);
265270
}
266271
return null;
267272
}
@@ -340,7 +345,7 @@ private static ContainerStatus getContainerStatus(PodStatus podStatus) {
340345
case 1:
341346
return statuses.get(0);
342347
default:
343-
final String containerId = ContainerUtil.getContainerId(ContainerUtil.CGROUP_FILE);
348+
final String containerId = ContainerUtil.getContainerId(cgroupPath);
344349
return containerId != null ? statuses.stream()
345350
.filter(cs -> cs.getContainerID().contains(containerId))
346351
.findFirst()
@@ -365,70 +370,67 @@ private static Container getContainer(PodSpec podSpec, String containerName) {
365370

366371
@Override
367372
public String lookup(final LogEvent event, final String key) {
368-
KubernetesInfo kubernetesInfo = null;
373+
KubernetesInfo info;
369374
READ_LOCK.lock();
370375
try {
371-
kubernetesInfo = KubernetesLookup.kubernetesInfo;
376+
info = kubernetesInfo;
372377
} finally {
373378
READ_LOCK.unlock();
374379
}
375-
if (kubernetesInfo == null) {
376-
return null;
377-
}
378380
if (key.startsWith(LABELS_PREFIX)) {
379-
return kubernetesInfo.labels != null ? kubernetesInfo.labels.get(key.substring(LABELS_PREFIX.length())) : null;
381+
return info.labels != null ? info.labels.get(key.substring(LABELS_PREFIX.length())) : null;
380382
}
381383
switch (key) {
382384
case ACCOUNT_NAME: {
383-
return kubernetesInfo.accountName;
385+
return info.accountName;
384386
}
385387
case ANNOTATIONS: {
386-
return kubernetesInfo.annotations != null ? kubernetesInfo.annotations.toString() : null;
388+
return info.annotations != null ? info.annotations.toString() : null;
387389
}
388390
case CONTAINER_ID: {
389-
return kubernetesInfo.containerId;
391+
return info.containerId;
390392
}
391393
case CONTAINER_NAME: {
392-
return kubernetesInfo.containerName;
394+
return info.containerName;
393395
}
394396
case HOST: {
395-
return kubernetesInfo.hostName;
397+
return info.hostName;
396398
}
397399
case HOST_IP: {
398-
return kubernetesInfo.hostIp;
400+
return info.hostIp;
399401
}
400402
case LABELS: {
401-
return kubernetesInfo.labels != null ? kubernetesInfo.labels.toString() : null;
403+
return info.labels != null ? info.labels.toString() : null;
402404
}
403405
case MASTER_URL: {
404-
return kubernetesInfo.masterUrl != null ? kubernetesInfo.masterUrl.toString() : null;
406+
return info.masterUrl != null ? info.masterUrl.toString() : null;
405407
}
406408
case NAMESPACE_ANNOTATIONS: {
407-
return kubernetesInfo.namespaceAnnotations != null ? kubernetesInfo.namespaceAnnotations.toString() : null;
409+
return info.namespaceAnnotations != null ? info.namespaceAnnotations.toString() : null;
408410
}
409411
case NAMESPACE_ID: {
410-
return kubernetesInfo.namespaceId;
412+
return info.namespaceId;
411413
}
412414
case NAMESPACE_LABELS: {
413-
return kubernetesInfo.namespaceLabels != null ? kubernetesInfo.namespaceLabels.toString() : null;
415+
return info.namespaceLabels != null ? info.namespaceLabels.toString() : null;
414416
}
415417
case NAMESPACE_NAME: {
416-
return kubernetesInfo.namespace;
418+
return info.namespace;
417419
}
418420
case POD_ID: {
419-
return kubernetesInfo.podId;
421+
return info.podId;
420422
}
421423
case POD_IP: {
422-
return kubernetesInfo.podIp;
424+
return info.podIp;
423425
}
424426
case POD_NAME: {
425-
return kubernetesInfo.podName;
427+
return info.podName;
426428
}
427429
case IMAGE_ID: {
428-
return kubernetesInfo.imageId;
430+
return info.imageId;
429431
}
430432
case IMAGE_NAME: {
431-
return kubernetesInfo.imageName;
433+
return info.imageName;
432434
}
433435
default:
434436
return null;
@@ -438,8 +440,9 @@ public String lookup(final LogEvent event, final String key) {
438440
/**
439441
* For unit testing only.
440442
*/
441-
static void clearInfo() {
443+
static void clear() {
442444
kubernetesInfo = null;
445+
cgroupPath = ContainerUtil.CGROUP_PATH;
443446
}
444447

445448
private static class KubernetesInfo {

‎log4j/src/test/java/io/fabric8/kubernetes/log4j/lookup/ContainerUtilTest.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,15 @@
2727

2828
class ContainerUtilTest {
2929

30-
private static final String CONTAINER_ID = "3dd988081e7149463c043b5d9c57d7309e079c5e9290f91feba1cc45a04d6a5b";
30+
// Also used in KubernetesLookupTest
31+
static final String CONTAINER_ID = "3dd988081e7149463c043b5d9c57d7309e079c5e9290f91feba1cc45a04d6a5b";
32+
static final String TEST_GOOD_CGROUP_RESOURCE = "cgroups/positive/case_0.dat";
33+
static final String TEST_BAD_CGROUP_RESOURCE = "cgroups/negative/case_0.dat";
3134

3235
static Stream<Arguments> should_recognize_container_id() {
3336
return Stream.of(
3437
// Some possible example /proc/self/cgroup
35-
Arguments.of("cgroups/positive/case_0.dat", CONTAINER_ID),
38+
Arguments.of(TEST_GOOD_CGROUP_RESOURCE, CONTAINER_ID),
3639
Arguments.of("cgroups/positive/case_1.dat", CONTAINER_ID),
3740
Arguments.of("cgroups/positive/case_2.dat", CONTAINER_ID),
3841
Arguments.of("cgroups/positive/case_3.dat", CONTAINER_ID),
@@ -42,7 +45,7 @@ static Stream<Arguments> should_recognize_container_id() {
4245
Arguments.of("cgroups/positive/case_7.dat", CONTAINER_ID),
4346
Arguments.of("cgroups/positive/case_8.dat", CONTAINER_ID),
4447
// Smoke test in case the file changes format
45-
Arguments.of("cgroups/negative/case_0.dat", null),
48+
Arguments.of(TEST_BAD_CGROUP_RESOURCE, null),
4649
Arguments.of("cgroups/negative/case_1.dat", null));
4750
}
4851

‎log4j/src/test/java/io/fabric8/kubernetes/log4j/lookup/KubernetesLookupTest.java‎

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616
package io.fabric8.kubernetes.log4j.lookup;
1717

1818
import com.fasterxml.jackson.databind.ObjectMapper;
19+
import io.fabric8.kubernetes.api.model.Container;
20+
import io.fabric8.kubernetes.api.model.ContainerBuilder;
21+
import io.fabric8.kubernetes.api.model.ContainerStatus;
22+
import io.fabric8.kubernetes.api.model.ContainerStatusBuilder;
1923
import io.fabric8.kubernetes.api.model.Namespace;
2024
import io.fabric8.kubernetes.api.model.NamespaceBuilder;
2125
import io.fabric8.kubernetes.api.model.Pod;
@@ -25,24 +29,30 @@
2529
import org.apache.logging.log4j.core.lookup.StrLookup;
2630
import org.junit.jupiter.api.AfterEach;
2731
import org.junit.jupiter.api.Test;
32+
import org.junit.jupiter.params.ParameterizedTest;
33+
import org.junit.jupiter.params.provider.MethodSource;
2834

2935
import java.net.URL;
36+
import java.nio.file.Paths;
37+
import java.util.stream.Stream;
3038

3139
import static org.assertj.core.api.Assertions.assertThat;
40+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
3241

3342
/**
3443
* Validate the Kubernetes Lookup.
3544
*/
3645
@EnableKubernetesMockClient(crud = true)
3746
class KubernetesLookupTest {
3847

48+
private static final int MAX_CONTAINER_COUNT = 2;
3949
private static KubernetesClient mockClient;
4050

4151
private static final ObjectMapper objectMapper = new ObjectMapper();
4252

4353
@AfterEach
4454
void cleanUp() {
45-
KubernetesLookup.clearInfo();
55+
KubernetesLookup.clear();
4656
}
4757

4858
@Test
@@ -100,6 +110,67 @@ void clusterPod() throws Exception {
100110
assertNamespaceLookups(lookup, namespace.getMetadata().getUid());
101111
}
102112

113+
static Stream<String> container_statuses_smoke_test() {
114+
return Stream.of(
115+
ContainerUtilTest.TEST_GOOD_CGROUP_RESOURCE,
116+
ContainerUtilTest.TEST_BAD_CGROUP_RESOURCE);
117+
}
118+
119+
@ParameterizedTest
120+
@MethodSource
121+
void container_statuses_smoke_test(String cgroupPath) throws Exception {
122+
Namespace namespace = new NamespaceBuilder().build();
123+
URL masterUrl = new URL("http://localhost:443/");
124+
// Supports any number of container statuses
125+
for (int containerCount = 0; containerCount <= MAX_CONTAINER_COUNT; containerCount++) {
126+
// If more than a status is available ContainerUtil is used, so we set a test file to scan for the container id
127+
KubernetesLookup.cgroupPath = Paths.get(KubernetesLookupTest.class.getClassLoader().getResource(cgroupPath).toURI());
128+
ContainerStatus[] containerStatuses = new ContainerStatus[containerCount];
129+
for (int i = 0; i < containerCount; i++) {
130+
containerStatuses[i] = new ContainerStatusBuilder().withName("container" + i)
131+
.withContainerID(i == 0 ? ContainerUtilTest.CONTAINER_ID : null).build();
132+
}
133+
Pod pod = new PodBuilder().withNewStatus()
134+
.withContainerStatuses(containerStatuses)
135+
.endStatus()
136+
.build();
137+
assertDoesNotThrow(() -> new KubernetesLookup(pod, namespace, masterUrl));
138+
cleanUp();
139+
}
140+
}
141+
142+
static Stream<String> containers_smoke_test() {
143+
return Stream.of(
144+
null,
145+
"container0");
146+
}
147+
148+
@ParameterizedTest
149+
@MethodSource
150+
void containers_smoke_test(String containerName) throws Exception {
151+
Namespace namespace = new NamespaceBuilder().build();
152+
URL masterUrl = new URL("http://localhost:443/");
153+
// Supports any number of containers in the spec
154+
for (int containerCount = 0; containerCount <= MAX_CONTAINER_COUNT; containerCount++) {
155+
Container[] containers = new Container[containerCount];
156+
for (int i = 0; i < containerCount; i++) {
157+
containers[i] = new ContainerBuilder().withName("container" + i).build();
158+
}
159+
Pod pod = new PodBuilder().withNewSpec()
160+
.withContainers(containers)
161+
.endSpec()
162+
.withNewStatus()
163+
.addNewContainerStatus()
164+
.withName(
165+
containerName)
166+
.endContainerStatus()
167+
.endStatus()
168+
.build();
169+
assertDoesNotThrow(() -> new KubernetesLookup(pod, namespace, masterUrl));
170+
cleanUp();
171+
}
172+
}
173+
103174
@Test
104175
void initialize_works_with_mock_client() {
105176
final Pod pod = mockClient.pods().resource(createPod()).create();
@@ -117,7 +188,12 @@ protected KubernetesClient createClient() {
117188

118189
@Test
119190
void no_client_should_return_no_data() {
120-
final StrLookup lookup = new KubernetesLookup();
191+
final StrLookup lookup = new KubernetesLookup() {
192+
@Override
193+
protected KubernetesClient createClient() {
194+
return null;
195+
}
196+
};
121197
assertThat(lookup.lookup("accountName")).isNull();
122198
}
123199

0 commit comments

Comments
 (0)