smolnar82 commented on code in PR #792:
URL: https://github.com/apache/knox/pull/792#discussion_r1319778438
##########
gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java:
##########
@@ -785,4 +784,20 @@ void failedToDiscoverClusterServices(String clusterName,
String topologyName,
@Message(level = MessageLevel.DEBUG,
text = "Request {0} is wrapped to url encoded form request.")
void wrappingRequestToUrlEncodedFormRequest(String requestURI);
+
+ @Message(level = MessageLevel.INFO,
+ text = "Checking gateway status. Deployed topologies: {0}. Waiting
for: {1}")
+ void checkingGatewayStatus(Set<String> deployedTopologies, Set<String>
missingTopologies);
+
+ @Message(level = MessageLevel.INFO,
+ text = "Checking gateway status. No topologies to check")
+ void checkingGatewayStatus();
Review Comment:
I'd use a more explicit log method name here.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/topology/impl/GatewayStatusService.java:
##########
@@ -0,0 +1,97 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.topology.impl;
+
+import java.io.File;
+import java.util.HashSet;
+import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.commons.io.FilenameUtils;
+import org.apache.knox.gateway.GatewayMessages;
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.i18n.messages.MessagesFactory;
+import org.apache.knox.gateway.services.Service;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+
+public class GatewayStatusService implements Service {
+ private static final GatewayMessages LOG =
MessagesFactory.get(GatewayMessages.class);
+ private final Set<String> deployedTopologies = new HashSet<>();
+ private Set<String> topologyNamesToCheck = new HashSet<>();
+ private GatewayConfig config;
+
+ public synchronized void onTopologyReady(String topologyName) {
+ deployedTopologies.add(topologyName);
+ }
+
+ public synchronized boolean status() {
+ if (topologyNamesToCheck.isEmpty()) {
+ LOG.checkingGatewayStatus();
+ return false;
+ }
+ Set<String> missing = new HashSet<>(topologyNamesToCheck);
+ missing.removeAll(deployedTopologies);
+ LOG.checkingGatewayStatus(deployedTopologies, missing);
+ return missing.isEmpty();
+ }
+
+ /**
+ * The list of topologies (which will be used to check the gateway status.)
+ * are either coming from the config or collected automatically.
+ * In the later case this should be called at startup, after the hadoop xml
resource parser
+ * already generated the descriptors from the hxr
+ */
+ public synchronized void initTopologiesToCheck() {
+ Set<String> healthCheckTopologies = config.getHealthCheckTopologies();
+ if (healthCheckTopologies.isEmpty()) {
+ topologyNamesToCheck = collectTopologies(config);
+ } else {
+ topologyNamesToCheck = healthCheckTopologies;
+ }
+ LOG.startingStatusMonitor(topologyNamesToCheck);
+ }
+
+ private Set<String> collectTopologies(GatewayConfig config) {
+ Set<String> result = new HashSet<>();
+ collectFiles(result, config.getGatewayTopologyDir(), ".xml");
+ collectFiles(result, config.getGatewayDescriptorsDir(), ".json");
+ LOG.collectedTopologiesForHealthCheck(result);
+ return result;
+ }
+
+ private void collectFiles(Set<String> result, String directory, String
extension) {
+ File[] files = new File(directory).getAbsoluteFile()
+ .listFiles((dir, name) ->
name.toLowerCase(Locale.ROOT).endsWith(extension));
+ if (files != null) {
+ for (File file : files) {
+ result.add(FilenameUtils.getBaseName(file.getName()));
+ }
+ }
+ }
+ @Override
+ public void init(GatewayConfig config, Map<String, String> options) throws
ServiceLifecycleException {
Review Comment:
I'd add a comment here as to why `initTopologiesToCheck` cannot be part of
the regular `init` method (resources are not yet ready at this point). To make
it clear to future readers.
##########
gateway-server/src/main/java/org/apache/knox/gateway/services/factory/GatewayStatusCheckerFactory.java:
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.factory;
+
+import static java.util.Arrays.asList;
+import static java.util.Collections.unmodifiableList;
+
+import java.util.Collection;
+import java.util.Map;
+
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.apache.knox.gateway.services.GatewayServices;
+import org.apache.knox.gateway.services.Service;
+import org.apache.knox.gateway.services.ServiceLifecycleException;
+import org.apache.knox.gateway.services.ServiceType;
+import org.apache.knox.gateway.services.topology.impl.GatewayStatusService;
+
+public class GatewayStatusCheckerFactory extends AbstractServiceFactory {
Review Comment:
This should be renamed to `GatewayStatusServiceFactory`.
##########
gateway-server/src/main/java/org/apache/knox/gateway/GatewayMessages.java:
##########
@@ -785,4 +784,20 @@ void failedToDiscoverClusterServices(String clusterName,
String topologyName,
@Message(level = MessageLevel.DEBUG,
text = "Request {0} is wrapped to url encoded form request.")
void wrappingRequestToUrlEncodedFormRequest(String requestURI);
+
+ @Message(level = MessageLevel.INFO,
+ text = "Checking gateway status. Deployed topologies: {0}. Waiting
for: {1}")
+ void checkingGatewayStatus(Set<String> deployedTopologies, Set<String>
missingTopologies);
+
+ @Message(level = MessageLevel.INFO,
+ text = "Checking gateway status. No topologies to check")
+ void checkingGatewayStatus();
+
+ @Message(level = MessageLevel.INFO,
+ text = "Collected topologies for health check: {0}")
+ void collectedTopologiesForHealthCheck(Set<String> result);
+
+ @Message(level = MessageLevel.INFO,
+ text = "Starting gateway status checker service. Topologies to
check: {0}")
Review Comment:
Maybe removing the word `checker`?
##########
gateway-server/src/test/java/org/apache/knox/gateway/services/topology/impl/GatewayStatusServiceTest.java:
##########
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.knox.gateway.services.topology.impl;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.util.Arrays;
+import java.util.HashSet;
+
+import org.apache.knox.gateway.config.GatewayConfig;
+import org.easymock.EasyMock;
+import org.junit.Test;
+
+public class GatewayStatusServiceTest {
+
+ @Test
+ public void testReadyStatus() throws Exception {
+ GatewayStatusService statusChecker = new GatewayStatusService();
Review Comment:
`statusChecker` -> `statusService`
##########
gateway-service-health/src/main/java/org/apache/knox/gateway/service/health/PingResource.java:
##########
@@ -81,7 +87,26 @@ private Response getPingResponse() {
}
String getPingContent() {
- return CONTENT;
+ return OK;
+ }
+
+ @GET
+ @Produces({TEXT_PLAIN})
+ @Path("gateway-status")
+ public Response status() {
+ response.setStatus(HttpServletResponse.SC_OK);
Review Comment:
The `response` field is always set to the same here and in `getPingResponse`
too. You may want to refactor this out to an `init` method and call it only
once (e.g. using the `PostConstruct` annotation as we do in other resource
classes).
##########
gateway-service-health/src/main/java/org/apache/knox/gateway/service/health/PingResource.java:
##########
@@ -81,7 +87,26 @@ private Response getPingResponse() {
}
String getPingContent() {
- return CONTENT;
+ return OK;
+ }
+
+ @GET
+ @Produces({TEXT_PLAIN})
+ @Path("gateway-status")
+ public Response status() {
+ response.setStatus(HttpServletResponse.SC_OK);
+ response.setHeader(CACHE_CONTROL, NO_CACHE);
+ response.setContentType(CONTENT_TYPE);
+ GatewayServices services = (GatewayServices) request.getServletContext()
+ .getAttribute(GatewayServices.GATEWAY_SERVICES_ATTRIBUTE);
+ GatewayStatusService statusChecker =
services.getService(ServiceType.GATEWAY_STATUS_SERVICE);
Review Comment:
`statusChecker` -> `statusService`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
