Skip to content

Commit a081d41

Browse files
authored
fix: properly read docker task config from service props & task props (#1616)
### Motivation In #1251 an issue regarding docker configs was resolved. The docker configuration was only read from the service props which is not used by default. The properties are only present in the task and not the service. This PR now fixes the issue and brings support to resolve the config from service & task properties. ### Modification Added a three step process to figure out which configuration to use when resolving properties that might be task / service specific. The order is now: Service -> Task (if present) -> Module Config ### Result The configuration is read correctly
1 parent 6224308 commit a081d41

File tree

2 files changed

+39
-10
lines changed

2 files changed

+39
-10
lines changed

modules/dockerized-services/impl/src/main/java/eu/cloudnetservice/modules/docker/impl/DockerizedLocalCloudServiceFactory.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.github.dockerjava.api.DockerClient;
2020
import eu.cloudnetservice.driver.event.EventManager;
2121
import eu.cloudnetservice.driver.language.I18n;
22+
import eu.cloudnetservice.driver.provider.ServiceTaskProvider;
2223
import eu.cloudnetservice.driver.registry.Service;
2324
import eu.cloudnetservice.driver.service.ServiceConfiguration;
2425
import eu.cloudnetservice.modules.docker.config.DockerConfiguration;
@@ -40,6 +41,7 @@ public class DockerizedLocalCloudServiceFactory extends BaseLocalCloudServiceFac
4041
protected final DefaultTickLoop mainThread;
4142
protected final EventManager eventManager;
4243
protected final DockerClient dockerClient;
44+
protected final ServiceTaskProvider serviceTaskProvider;
4345
protected final DockerConfiguration dockerConfiguration;
4446
protected final CloudServiceManager cloudServiceManager;
4547

@@ -52,6 +54,7 @@ public DockerizedLocalCloudServiceFactory(
5254
@NonNull EventManager eventManager,
5355
@NonNull ServiceVersionProvider versionProvider,
5456
@NonNull DockerClient dockerClient,
57+
@NonNull ServiceTaskProvider serviceTaskProvider,
5558
@NonNull DockerConfiguration configuration
5659
) {
5760
super(nodeConfig, versionProvider);
@@ -60,6 +63,7 @@ public DockerizedLocalCloudServiceFactory(
6063
this.eventManager = eventManager;
6164
this.cloudServiceManager = cloudServiceManager;
6265
this.dockerClient = dockerClient;
66+
this.serviceTaskProvider = serviceTaskProvider;
6367
this.dockerConfiguration = configuration;
6468
}
6569

@@ -82,6 +86,7 @@ public DockerizedLocalCloudServiceFactory(
8286
this.eventManager,
8387
this.versionProvider,
8488
preparer,
89+
this.serviceTaskProvider,
8590
this.dockerClient,
8691
this.dockerConfiguration);
8792
}

modules/dockerized-services/impl/src/main/java/eu/cloudnetservice/modules/docker/impl/DockerizedService.java

Lines changed: 34 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,13 @@
3131
import com.github.dockerjava.api.model.LogConfig;
3232
import com.github.dockerjava.api.model.RestartPolicy;
3333
import com.github.dockerjava.api.model.Volume;
34+
import eu.cloudnetservice.driver.document.property.DocProperty;
3435
import eu.cloudnetservice.driver.event.EventManager;
3536
import eu.cloudnetservice.driver.language.I18n;
37+
import eu.cloudnetservice.driver.provider.ServiceTaskProvider;
3638
import eu.cloudnetservice.driver.registry.Service;
3739
import eu.cloudnetservice.driver.service.ServiceConfiguration;
40+
import eu.cloudnetservice.driver.service.ServiceTask;
3841
import eu.cloudnetservice.modules.docker.config.DockerConfiguration;
3942
import eu.cloudnetservice.modules.docker.config.DockerImage;
4043
import eu.cloudnetservice.modules.docker.config.DockerPortMapping;
@@ -86,7 +89,11 @@ public class DockerizedService extends JVMService {
8689
Capability.DAC_OVERRIDE,
8790
Capability.NET_BIND_SERVICE
8891
).toArray(Capability[]::new);
92+
protected static final DocProperty<TaskDockerConfig> TASK_DOCKER_CONFIG = DocProperty.property(
93+
"dockerConfig",
94+
TaskDockerConfig.class);
8995

96+
protected final ServiceTask selfTask;
9097
protected final DockerClient dockerClient;
9198
protected final DockerConfiguration configuration;
9299

@@ -105,6 +112,7 @@ protected DockerizedService(
105112
@NonNull EventManager eventManager,
106113
@NonNull ServiceVersionProvider versionProvider,
107114
@NonNull ServiceConfigurationPreparer serviceConfigurationPreparer,
115+
@NonNull ServiceTaskProvider serviceTaskProvider,
108116
@NonNull DockerClient dockerClient,
109117
@NonNull DockerConfiguration dockerConfiguration
110118
) {
@@ -120,6 +128,7 @@ protected DockerizedService(
120128
versionProvider,
121129
serviceConfigurationPreparer);
122130

131+
this.selfTask = serviceTaskProvider.serviceTask(configuration.serviceId().taskName());
123132
this.dockerClient = dockerClient;
124133
this.configuration = dockerConfiguration;
125134
}
@@ -170,16 +179,17 @@ protected void doStartProcess(
170179
var user = Objects.requireNonNullElse(this.configuration.user(), "");
171180

172181
// get the task specific options
182+
var taskConfig = this.resolveTaskDockerConfig();
173183
var image = Objects.requireNonNullElse(
174-
this.readFromTaskConfig(TaskDockerConfig::javaImage),
184+
this.readFromTaskConfig(taskConfig, TaskDockerConfig::javaImage),
175185
this.configuration.javaImage());
176186
var taskExposedPorts = Objects.requireNonNullElse(
177-
this.readFromTaskConfig(TaskDockerConfig::exposedPorts),
187+
this.readFromTaskConfig(taskConfig, TaskDockerConfig::exposedPorts),
178188
Set.<DockerPortMapping>of());
179189

180190
// combine the task options with the global options
181-
var volumes = this.collectVolumes();
182-
var binds = this.collectBinds(wrapperPath);
191+
var volumes = this.collectVolumes(taskConfig);
192+
var binds = this.collectBinds(wrapperPath, taskConfig);
183193
var exposedPorts = Stream.of(taskExposedPorts, this.configuration.exposedPorts())
184194
.flatMap(Collection::stream)
185195
.map(portMapping -> {
@@ -309,7 +319,7 @@ public void doDelete() {
309319
}
310320
}
311321

312-
protected @NonNull Bind[] collectBinds(@NonNull Path wrapperFilePath) {
322+
protected @NonNull Bind[] collectBinds(@NonNull Path wrapperFilePath, @Nullable TaskDockerConfig config) {
313323
Set<Bind> binds = new HashSet<>();
314324

315325
// allow the container full access to the work directory and the wrapper file
@@ -319,7 +329,9 @@ public void doDelete() {
319329
binds.add(this.bindFromPath(this.serviceDirectory.toAbsolutePath().toString(), AccessMode.rw));
320330

321331
// get the task specific volumes and concat them with the default volumes
322-
var taskBinds = Objects.requireNonNullElse(this.readFromTaskConfig(TaskDockerConfig::binds), Set.<String>of());
332+
var taskBinds = Objects.requireNonNullElse(
333+
this.readFromTaskConfig(config, TaskDockerConfig::binds),
334+
Set.<String>of());
323335
binds.addAll(Stream.concat(taskBinds.stream(), this.configuration.binds().stream())
324336
.map(path -> this.serviceDirectory.resolve(path).toAbsolutePath().toString())
325337
.map(path -> this.bindFromPath(path, AccessMode.rw))
@@ -329,16 +341,28 @@ public void doDelete() {
329341
return binds.toArray(Bind[]::new);
330342
}
331343

332-
protected @NonNull Volume[] collectVolumes() {
333-
var taskVolumes = Objects.requireNonNullElse(this.readFromTaskConfig(TaskDockerConfig::volumes), Set.<String>of());
344+
protected @NonNull Volume[] collectVolumes(@Nullable TaskDockerConfig config) {
345+
var taskVolumes = Objects.requireNonNullElse(
346+
this.readFromTaskConfig(config, TaskDockerConfig::volumes),
347+
Set.<String>of());
334348
return Stream.concat(this.configuration.volumes().stream(), taskVolumes.stream())
335349
.map(Volume::new)
336350
.distinct()
337351
.toArray(Volume[]::new);
338352
}
339353

340-
protected @Nullable <T> T readFromTaskConfig(@NonNull Function<TaskDockerConfig, T> reader) {
341-
var config = this.serviceConfiguration.propertyHolder().readObject("dockerConfig", TaskDockerConfig.class);
354+
protected @Nullable TaskDockerConfig resolveTaskDockerConfig() {
355+
return this.serviceConfiguration.readPropertyOrGet(
356+
TASK_DOCKER_CONFIG,
357+
// it is possible to start a service with a task name that is not linked to a real task therefore we need to
358+
// make sure that the task exists before proceeding
359+
() -> this.selfTask == null ? null : this.selfTask.readProperty(TASK_DOCKER_CONFIG));
360+
}
361+
362+
protected @Nullable <T> T readFromTaskConfig(
363+
@Nullable TaskDockerConfig config,
364+
@NonNull Function<TaskDockerConfig, T> reader
365+
) {
342366
return config == null ? null : reader.apply(config);
343367
}
344368

0 commit comments

Comments
 (0)