Skip to content

Commit 48b40d4

Browse files
fmichielssenfmichielssen
committed
Merge pull request 'Fix #25531: add option to allow recovering apps from different config' (#53) from feature/25758 into develop
2 parents 15aa072 + 41937e2 commit 48b40d4

6 files changed

Lines changed: 73 additions & 25 deletions

File tree

‎src/main/java/eu/openanalytics/containerproxy/backend/AbstractContainerBackend.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.UserGroupsKey;
4444
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.UserIdKey;
4545
import eu.openanalytics.containerproxy.model.spec.ContainerSpec;
46+
import eu.openanalytics.containerproxy.service.AppRecoveryService;
4647
import eu.openanalytics.containerproxy.service.IdentifierService;
4748
import eu.openanalytics.containerproxy.service.UserService;
4849
import eu.openanalytics.containerproxy.spec.expression.ExpressionAwareContainerSpec;
@@ -105,6 +106,11 @@ public abstract class AbstractContainerBackend implements IContainerBackend {
105106
@Inject
106107
protected IdentifierService identifierService;
107108

109+
@Inject
110+
@Lazy
111+
// Note: lazy to prevent cyclic dependencies
112+
protected AppRecoveryService appRecoveryService;
113+
108114
@Override
109115
public void initialize() throws ContainerProxyException {
110116
// If this application runs as a container itself, things like port publishing can be omitted.

‎src/main/java/eu/openanalytics/containerproxy/backend/docker/AbstractDockerBackend.java‎

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import eu.openanalytics.containerproxy.backend.AbstractContainerBackend;
3333
import eu.openanalytics.containerproxy.model.runtime.Container;
3434
import eu.openanalytics.containerproxy.model.runtime.Proxy;
35-
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.InstanceIdKey;
3635
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.RuntimeValue;
3736
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.RuntimeValueKey;
3837
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.RuntimeValueKeyRegistry;
@@ -122,15 +121,14 @@ public void setupPortMappingExistingProxy(Proxy proxy, Container container, Map<
122121
for (String mappingKey : container.getSpec().getPortMapping().keySet()) {
123122
int containerPort = container.getSpec().getPortMapping().get(mappingKey);
124123

125-
int servicePort = -1; // in case of internal networking
124+
int hostPort = -1; // in case of internal networking
126125
if (portBindings.containsKey(containerPort) && portBindings.get(containerPort) != 0) {
127126
// in case of non internal networking
128-
servicePort = portBindings.get(containerPort);
129-
portAllocator.addExistingPort(proxy.getUserId(), servicePort);
127+
hostPort = portBindings.get(containerPort);
130128
}
131129

132130
String mapping = mappingStrategy.createMapping(mappingKey, container, proxy);
133-
URI target = calculateTarget(container, containerPort, servicePort);
131+
URI target = calculateTarget(container, containerPort, hostPort);
134132
proxy.getTargets().put(mapping, target);
135133
}
136134
}
@@ -145,14 +143,9 @@ protected List<String> convertEnv(Map<String, String> env) {
145143
return res;
146144
}
147145

148-
protected Map<RuntimeValueKey<?>, RuntimeValue> parseLabelsAsRuntimeValues(String containerId, ImmutableMap<String, String> labels) {
149-
if (labels == null) {
150-
return null;
151-
}
152146

153-
String containerInstanceId = labels.get(InstanceIdKey.inst.getKeyAsLabel());
154-
if (containerInstanceId == null || !containerInstanceId.equals(identifierService.instanceId)) {
155-
log.warn("Ignoring container {} because instanceId {} is not correct", containerId, containerInstanceId);
147+
protected Map<RuntimeValueKey<?>, RuntimeValue> parseLabelsAsRuntimeValues(String containerId, ImmutableMap<String, String> labels) {
148+
if (labels == null) {
156149
return null;
157150
}
158151

‎src/main/java/eu/openanalytics/containerproxy/backend/docker/DockerEngineBackend.java‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@
3232
import eu.openanalytics.containerproxy.model.runtime.Container;
3333
import eu.openanalytics.containerproxy.model.runtime.ExistingContainerInfo;
3434
import eu.openanalytics.containerproxy.model.runtime.Proxy;
35+
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.InstanceIdKey;
3536
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.RuntimeValue;
3637
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.RuntimeValueKey;
38+
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.UserIdKey;
3739
import eu.openanalytics.containerproxy.model.spec.ContainerSpec;
3840

3941
import java.net.URI;
@@ -165,7 +167,7 @@ protected void doStopProxy(Proxy proxy) throws Exception {
165167

166168
@Override
167169
public List<ExistingContainerInfo> scanExistingContainers() throws Exception {
168-
ArrayList<ExistingContainerInfo> containers = new ArrayList<ExistingContainerInfo>();
170+
ArrayList<ExistingContainerInfo> containers = new ArrayList<>();
169171

170172
for (com.spotify.docker.client.messages.Container container: dockerClient.listContainers(ListContainersParam.allContainers())) {
171173
if (!container.state().equalsIgnoreCase("running")) {
@@ -177,14 +179,25 @@ public List<ExistingContainerInfo> scanExistingContainers() throws Exception {
177179
continue;
178180
}
179181

182+
// add ports to PortAllocator (even if we don't recover the proxy)
183+
for (PortMapping portMapping: container.ports()) {
184+
portAllocator.addExistingPort(runtimeValues.get(UserIdKey.inst).getValue(), portMapping.publicPort());
185+
}
186+
187+
String containerInstanceId = runtimeValues.get(InstanceIdKey.inst).getValue();
188+
if (!appRecoveryService.canRecoverProxy(containerInstanceId)) {
189+
log.warn("Ignoring container {} because instanceId {} is not correct", container.id(), containerInstanceId);
190+
continue;
191+
}
192+
180193
Map<Integer, Integer> portBindings = new HashMap<>();
181194
for (PortMapping portMapping: container.ports()) {
182195
int hostPort = portMapping.publicPort();
183196
int containerPort = portMapping.privatePort();
184197
portBindings.put(containerPort, hostPort);
185198
}
186199

187-
containers.add(new ExistingContainerInfo(container.id(), runtimeValues, container.image(), portBindings, new HashMap()));
200+
containers.add(new ExistingContainerInfo(container.id(), runtimeValues, container.image(), portBindings, new HashMap<>()));
188201

189202
}
190203

‎src/main/java/eu/openanalytics/containerproxy/backend/docker/DockerSwarmBackend.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import eu.openanalytics.containerproxy.model.runtime.Container;
3535
import eu.openanalytics.containerproxy.model.runtime.ExistingContainerInfo;
3636
import eu.openanalytics.containerproxy.model.runtime.Proxy;
37+
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.InstanceIdKey;
3738
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.RuntimeValue;
3839
import eu.openanalytics.containerproxy.model.runtime.runtimevalues.RuntimeValueKey;
3940
import eu.openanalytics.containerproxy.model.spec.ContainerSpec;
@@ -190,7 +191,7 @@ protected void doStopProxy(Proxy proxy) throws Exception {
190191

191192
@Override
192193
public List<ExistingContainerInfo> scanExistingContainers() throws Exception {
193-
ArrayList<ExistingContainerInfo> containers = new ArrayList<ExistingContainerInfo>();
194+
ArrayList<ExistingContainerInfo> containers = new ArrayList<>();
194195

195196
for (Service service : dockerClient.listServices()) {
196197
com.spotify.docker.client.messages.swarm.ContainerSpec containerSpec = service.spec().taskTemplate().containerSpec();
@@ -210,6 +211,12 @@ public List<ExistingContainerInfo> scanExistingContainers() throws Exception {
210211
continue;
211212
}
212213

214+
String containerInstanceId = runtimeValues.get(InstanceIdKey.inst).getValue();
215+
if (!appRecoveryService.canRecoverProxy(containerInstanceId)) {
216+
log.warn("Ignoring container {} because instanceId {} is not correct", containersInService.get(0).id(), containerInstanceId);
217+
continue;
218+
}
219+
213220
Map<Integer, Integer> portBindings = new HashMap<>();
214221
if (service.endpoint() != null && service.endpoint().ports() != null){
215222
for (PortConfig portMapping : service.endpoint().ports()) {

‎src/main/java/eu/openanalytics/containerproxy/backend/kubernetes/KubernetesBackend.java‎

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,6 @@ public List<ExistingContainerInfo> scanExistingContainers() {
508508
for (String namespace : namespaces) {
509509
List<Pod> pods = kubeClient.pods().inNamespace(namespace)
510510
.withLabel(ProxiedAppKey.inst.getKeyAsLabel(), "true")
511-
.withLabel(InstanceIdKey.inst.getKeyAsLabel(), identifierService.instanceId)
512511
.list().getItems();
513512

514513
for (Pod pod : pods) {
@@ -529,6 +528,12 @@ public List<ExistingContainerInfo> scanExistingContainers() {
529528
continue;
530529
}
531530

531+
String containerInstanceId = runtimeValues.get(InstanceIdKey.inst).getValue();
532+
if (!appRecoveryService.canRecoverProxy(containerInstanceId)) {
533+
log.warn("Ignoring container {} because instanceId {} is not correct", containerId, containerInstanceId);
534+
continue;
535+
}
536+
532537
Map<Integer, Integer> portBindings = new HashMap<>();
533538
for (ContainerPort portMapping: pod.getSpec().getContainers().get(0).getPorts()) {
534539
Integer hostPort = portMapping.getHostPort();
@@ -556,13 +561,6 @@ public List<ExistingContainerInfo> scanExistingContainers() {
556561
private Map<RuntimeValueKey<?>, RuntimeValue> parseLabelsAndAnnotationsAsRuntimeValues(String containerId,
557562
Map<String, String> labels,
558563
Map<String, String> annotations) {
559-
560-
String containerInstanceId = labels.get(InstanceIdKey.inst.getKeyAsLabel());
561-
if (containerInstanceId == null || !containerInstanceId.equals(identifierService.instanceId)) {
562-
log.warn("Ignoring container {} because instanceId {} is not correct", containerId, containerInstanceId);
563-
return null;
564-
}
565-
566564
Map<RuntimeValueKey<?>, RuntimeValue> runtimeValues = new HashMap<>();
567565

568566
for (RuntimeValueKey<?> key : RuntimeValueKeyRegistry.getRuntimeValueKeys()) {

‎src/main/java/eu/openanalytics/containerproxy/service/AppRecoveryService.java‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.springframework.stereotype.Service;
4343

4444
import javax.inject.Inject;
45+
import javax.ws.rs.HEAD;
4546
import java.util.HashMap;
4647
import java.util.Map;
4748

@@ -54,6 +55,7 @@
5455
public class AppRecoveryService {
5556

5657
protected static final String PROPERTY_RECOVER_RUNNING_PROXIES = "proxy.recover_running_proxies";
58+
protected static final String PROPERTY_RECOVER_RUNNING_PROXIES_FROM_DIFFERENT_CONFIG = "proxy.recover_running_proxies_from_different_config";
5759

5860
private final Logger log = LogManager.getLogger(AppRecoveryService.class);
5961

@@ -75,14 +77,25 @@ public class AppRecoveryService {
7577
@Inject
7678
private SpecExpressionResolver expressionResolver;
7779

80+
@Inject
81+
private IdentifierService identifierService;
82+
7883
private boolean isReady = false;
7984

85+
private boolean recoverFromDifferentConfig;
86+
8087
@EventListener(ApplicationReadyEvent.class)
8188
public void recoverRunningApps() throws Exception {
8289
if (Boolean.parseBoolean(environment.getProperty(PROPERTY_RECOVER_RUNNING_PROXIES, "false"))) {
83-
log.info("Recovery of running apps enabled");
90+
recoverFromDifferentConfig = Boolean.parseBoolean(environment.getProperty(PROPERTY_RECOVER_RUNNING_PROXIES_FROM_DIFFERENT_CONFIG, "false"));
8491

85-
Map<String, Proxy> proxies = new HashMap();
92+
if (recoverFromDifferentConfig) {
93+
log.info("Recovery of running apps enabled (even apps started with a different config file)");
94+
} else {
95+
log.info("Recovery of running apps enabled (but only apps started with the current config file)");
96+
}
97+
98+
Map<String, Proxy> proxies = new HashMap<>();
8699

87100
for (ExistingContainerInfo containerInfo: containerBackend.scanExistingContainers()) {
88101
String proxyId = containerInfo.getRuntimeValue(ProxyIdKey.inst).getValue();
@@ -149,4 +162,22 @@ public boolean isReady() {
149162
return isReady;
150163
}
151164

165+
/**
166+
* Checks whether the proxy with the given instanceId may be recovered in this ShinyProxy server.
167+
*/
168+
public Boolean canRecoverProxy(String containerInstanceId) {
169+
if (containerInstanceId == null) {
170+
// sanity check
171+
return false;
172+
}
173+
174+
if (recoverFromDifferentConfig) {
175+
// always allow to recover
176+
return true;
177+
}
178+
179+
// only allow if instanceId is equal to the current instanceId
180+
return containerInstanceId.equals(identifierService.instanceId);
181+
}
182+
152183
}

0 commit comments

Comments
 (0)