From aacdf3803fe4680c203af82456767ea60a8b756d Mon Sep 17 00:00:00 2001 From: Rudi Schlatte Date: Wed, 20 Mar 2024 09:59:28 +0100 Subject: [PATCH] Fix crash in initial app message, refine logging Change-Id: Ibbed4e81566ffbf5f7723196153f2167466a0f27 --- .../optimiser/controller/ExnConnector.java | 32 ++++++++----------- .../controller/NebulousAppDeployer.java | 8 ++--- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/optimiser-controller/src/main/java/eu/nebulouscloud/optimiser/controller/ExnConnector.java b/optimiser-controller/src/main/java/eu/nebulouscloud/optimiser/controller/ExnConnector.java index 1ba915e..c076be2 100644 --- a/optimiser-controller/src/main/java/eu/nebulouscloud/optimiser/controller/ExnConnector.java +++ b/optimiser-controller/src/main/java/eu/nebulouscloud/optimiser/controller/ExnConnector.java @@ -188,8 +188,8 @@ public class ExnConnector { @Override public void onMessage(String key, String address, Map body, Message message, Context context) { try { - String app_id = message.property("application").toString(); // should be string already, but don't want to cast - if (app_id == null) app_id = message.subject(); // TODO: remove for second version, leaving it in just to be safe + Object app_id = message.property("application"); // might be null + if (app_id == null) app_id = message.subject(); // if app_id is still null, the filename will look a bit funky but it's not a problem log.info("App creation message received", keyValue("appId", app_id)); JsonNode appMessage = mapper.valueToTree(body); @@ -258,10 +258,11 @@ public class ExnConnector { * * @param responseMessage The response from exn-middleware. * @param appID The application ID, used for logging only. + * @param caller Caller information, used for logging only. * @return The SAL response as a parsed JsonNode, or a node where {@code * isMissingNode()} will return true if SAL reported an error. */ - private static JsonNode extractPayloadFromExnResponse(Map responseMessage, String appID) { + private static JsonNode extractPayloadFromExnResponse(Map responseMessage, String appID, String caller) { JsonNode response = mapper.valueToTree(responseMessage); String salRawResponse = response.at("/body").asText(); // it's already a string, asText() is for the type system JsonNode metadata = response.at("/metaData"); @@ -270,7 +271,7 @@ public class ExnConnector { salResponse = mapper.readTree(salRawResponse); } catch (JsonProcessingException e) { log.error("Could not read message body as JSON: body = '{}'", salRawResponse, - keyValue("appId", appID), e); + keyValue("appId", appID), keyValue("caller", caller), e); return mapper.missingNode(); } if (!metadata.at("/status").asText().startsWith("2")) { @@ -278,7 +279,7 @@ public class ExnConnector { log.error("exn-middleware-sal request failed with error code '{}' and message '{}'", metadata.at("/status"), salResponse.at("/message").asText(), - keyValue("appId", appID)); + keyValue("appId", appID), keyValue("caller", caller)); return mapper.missingNode(); } return salResponse; @@ -373,7 +374,7 @@ public class ExnConnector { return null; } Map response = findSalNodeCandidates.sendSync(msg, appID, null, false); - JsonNode payload = extractPayloadFromExnResponse(response, appID); + JsonNode payload = extractPayloadFromExnResponse(response, appID, "findNodeCandidatesFromSal"); if (payload.isMissingNode()) return null; if (!payload.isArray()) return null; List candidates = Arrays.asList(mapper.convertValue(payload, NodeCandidate[].class)); @@ -433,7 +434,7 @@ public class ExnConnector { return false; } Map response = defineCluster.sendSync(msg, appID, null, false); - JsonNode payload = extractPayloadFromExnResponse(response, appID); + JsonNode payload = extractPayloadFromExnResponse(response, appID, "defineCluster"); return payload.asBoolean(); } @@ -446,7 +447,7 @@ public class ExnConnector { public JsonNode getCluster(String appID) { Map msg = Map.of("metaData", Map.of("user", "admin", "clusterName", appID)); Map response = getCluster.sendSync(msg, appID, null, false); - JsonNode payload = extractPayloadFromExnResponse(response, appID); + JsonNode payload = extractPayloadFromExnResponse(response, appID, "getCluster"); return payload.isMissingNode() ? null : payload; } @@ -468,7 +469,7 @@ public class ExnConnector { return false; } Map response = labelNodes.sendSync(msg, appID, null, false); - JsonNode payload = extractPayloadFromExnResponse(response, appID); + JsonNode payload = extractPayloadFromExnResponse(response, appID, "labelNodes"); return payload.isMissingNode() ? false : true; } @@ -484,7 +485,7 @@ public class ExnConnector { Map msg = Map.of("metaData", Map.of("user", "admin", "clusterName", clusterName)); Map response = deployCluster.sendSync(msg, appID, null, false); - JsonNode payload = extractPayloadFromExnResponse(response, appID); + JsonNode payload = extractPayloadFromExnResponse(response, appID, "deployCluster"); return payload.asBoolean(); } @@ -515,7 +516,7 @@ public class ExnConnector { return -1; } Map response = deployApplication.sendSync(msg, appID, null, false); - JsonNode payload = extractPayloadFromExnResponse(response, appID); + JsonNode payload = extractPayloadFromExnResponse(response, appID, "deployApplication"); return payload.asLong(); } @@ -545,7 +546,7 @@ public class ExnConnector { } Map response = scaleOut.sendSync(msg, appID, null, false); // Called for side-effect only; we want to log errors - JsonNode payload = extractPayloadFromExnResponse(response, appID); + JsonNode payload = extractPayloadFromExnResponse(response, appID, "scaleOut"); } /** @@ -570,13 +571,8 @@ public class ExnConnector { return false; } Map response = scaleIn.sendSync(msg, appID, null, false); - JsonNode payload = extractPayloadFromExnResponse(response, appID); + JsonNode payload = extractPayloadFromExnResponse(response, appID, "scaleIn"); return payload.asBoolean(); - // TODO: check if we still need to unwrap this; see - // `AbstractProcessor.groovy#normalizeResponse` and bug 2055053 - // https://opendev.org/nebulous/exn-middleware/src/commit/ffc2ca7bdf657b3831d2b803ff2b84d5e8e1bdcd/exn-middleware-core/src/main/groovy/eu/nebulouscloud/exn/modules/sal/processors/AbstractProcessor.groovy#L111 - // https://bugs.launchpad.net/nebulous/+bug/2055053 - // return payload.at("/success").asBoolean(); } diff --git a/optimiser-controller/src/main/java/eu/nebulouscloud/optimiser/controller/NebulousAppDeployer.java b/optimiser-controller/src/main/java/eu/nebulouscloud/optimiser/controller/NebulousAppDeployer.java index 7a1d844..3de9baa 100644 --- a/optimiser-controller/src/main/java/eu/nebulouscloud/optimiser/controller/NebulousAppDeployer.java +++ b/optimiser-controller/src/main/java/eu/nebulouscloud/optimiser/controller/NebulousAppDeployer.java @@ -188,7 +188,7 @@ public class NebulousAppDeployer { // 3. Select node candidates // Controller node - log.debug("Deciding on controller node candidate", keyValue("appId", appUUID)); + log.info("Deciding on controller node candidate", keyValue("appId", appUUID)); String masterNodeName = clusterName + "-masternode"; // safe because all component node names end with a number NodeCandidate masterNodeCandidate = null; if (controllerCandidates.size() > 0) { @@ -204,7 +204,7 @@ public class NebulousAppDeployer { } // Component nodes - log.debug("Collecting worker nodes for {}", appUUID, keyValue("appId", appUUID)); + log.info("Collecting worker nodes for {}", appUUID, keyValue("appId", appUUID)); ArrayNode nodeLabels = mapper.createArrayNode(); Map clusterNodes = new HashMap<>();; // Here we collect multiple things: @@ -220,7 +220,7 @@ public class NebulousAppDeployer { Set nodeNames = new HashSet<>(); List candidates = workerCandidates.get(componentName); if (candidates.size() == 0) { - log.error("Empty node candidate list for component ~s, continuing without creating node", componentName, keyValue("appId", appUUID)); + log.error("Empty node candidate list for component {}, continuing without creating node", componentName, keyValue("appId", appUUID)); continue; } for (int nodeNumber = 1; nodeNumber <= numberOfNodes; nodeNumber++) { @@ -425,7 +425,7 @@ public class NebulousAppDeployer { keyValue("appId", appUUID)); List candidates = conn.findNodeCandidates(newR, appUUID); if (candidates.size() == 0) { - log.error("Empty node candidate list for component ~s, continuing without creating node", componentName, + log.error("Empty node candidate list for component {}, continuing without creating node", componentName, keyValue("appId", appUUID)); continue; }