Skip to content

Commit 96e42a9

Browse files
Locharla, SandeepLocharla, Sandeep
Locharla, Sandeep
authored and
Locharla, Sandeep
committed
Fixed the initial review comments
1 parent 8f3794a commit 96e42a9

File tree

5 files changed

+170
-6
lines changed

5 files changed

+170
-6
lines changed

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/feign/FeignConfiguration.java‎

Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.cloudstack.storage.feign;
21+
22+
import com.fasterxml.jackson.databind.ObjectMapper;
23+
import feign.RequestInterceptor;
24+
import feign.Retryer;
25+
import feign.Client;
26+
import feign.httpclient.ApacheHttpClient;
27+
import feign.codec.Decoder;
28+
import feign.codec.Encoder;
29+
import feign.Response;
30+
import feign.codec.DecodeException;
31+
import feign.codec.EncodeException;
32+
import com.fasterxml.jackson.core.JsonProcessingException;
33+
import com.fasterxml.jackson.databind.DeserializationFeature;
34+
import org.apache.http.conn.ConnectionKeepAliveStrategy;
35+
import org.apache.http.conn.ssl.NoopHostnameVerifier;
36+
import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
37+
import org.apache.http.conn.ssl.TrustAllStrategy;
38+
import org.apache.http.impl.client.CloseableHttpClient;
39+
import org.apache.http.impl.client.HttpClientBuilder;
40+
import org.apache.http.ssl.SSLContexts;
41+
import org.apache.logging.log4j.LogManager;
42+
import org.apache.logging.log4j.Logger;
43+
44+
import javax.net.ssl.SSLContext;
45+
import java.io.IOException;
46+
import java.io.InputStream;
47+
import java.lang.reflect.Type;
48+
import java.nio.charset.StandardCharsets;
49+
import java.util.concurrent.TimeUnit;
50+
51+
public class FeignConfiguration {
52+
private static final Logger logger = LogManager.getLogger(FeignConfiguration.class);
53+
54+
private final int retryMaxAttempt = 3;
55+
private final int retryMaxInterval = 5;
56+
private final String ontapFeignMaxConnection = "80";
57+
private final String ontapFeignMaxConnectionPerRoute = "20";
58+
private final ObjectMapper objectMapper;
59+
60+
public FeignConfiguration() {
61+
this.objectMapper = new ObjectMapper();
62+
this.objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
63+
}
64+
65+
public Client createClient() {
66+
int maxConn;
67+
int maxConnPerRoute;
68+
try {
69+
maxConn = Integer.parseInt(this.ontapFeignMaxConnection);
70+
} catch (Exception e) {
71+
logger.error("ontapFeignClient: parse max connection failed, using default");
72+
maxConn = 20;
73+
}
74+
try {
75+
maxConnPerRoute = Integer.parseInt(this.ontapFeignMaxConnectionPerRoute);
76+
} catch (Exception e) {
77+
logger.error("ontapFeignClient: parse max connection per route failed, using default");
78+
maxConnPerRoute = 2;
79+
}
80+
logger.debug("ontapFeignClient: maxConn={}, maxConnPerRoute={}", maxConn, maxConnPerRoute);
81+
ConnectionKeepAliveStrategy keepAliveStrategy = (response, context) -> 0;
82+
CloseableHttpClient httpClient = HttpClientBuilder.create()
83+
.setMaxConnTotal(maxConn)
84+
.setMaxConnPerRoute(maxConnPerRoute)
85+
.setKeepAliveStrategy(keepAliveStrategy)
86+
.setSSLSocketFactory(getSSLSocketFactory())
87+
.setConnectionTimeToLive(60, TimeUnit.SECONDS)
88+
.build();
89+
return new ApacheHttpClient(httpClient);
90+
}
91+
92+
private SSLConnectionSocketFactory getSSLSocketFactory() {
93+
try {
94+
SSLContext sslContext = SSLContexts.custom().loadTrustMaterial(null, new TrustAllStrategy()).build();
95+
return new SSLConnectionSocketFactory(sslContext, new NoopHostnameVerifier());
96+
} catch (Exception ex) {
97+
throw new RuntimeException(ex);
98+
}
99+
}
100+
101+
public RequestInterceptor createRequestInterceptor() {
102+
return template -> {
103+
logger.info("Feign Request URL: {}", template.url());
104+
logger.info("HTTP Method: {}", template.method());
105+
logger.trace("Headers: {}", template.headers());
106+
if (template.body() != null) {
107+
logger.info("Body: {}", new String(template.body(), StandardCharsets.UTF_8));
108+
}
109+
};
110+
}
111+
112+
public Retryer createRetryer() {
113+
return new Retryer.Default(1000L, retryMaxInterval * 1000L, retryMaxAttempt);
114+
}
115+
116+
public Encoder createEncoder() {
117+
return new Encoder() {
118+
@Override
119+
public void encode(Object object, Type bodyType, feign.RequestTemplate template) throws EncodeException {
120+
if (object == null) {
121+
template.body(null, StandardCharsets.UTF_8);
122+
return;
123+
}
124+
try {
125+
byte[] jsonBytes = objectMapper.writeValueAsBytes(object);
126+
template.body(jsonBytes, StandardCharsets.UTF_8);
127+
template.header("Content-Type", "application/json");
128+
} catch (JsonProcessingException e) {
129+
throw new EncodeException("Error encoding object to JSON", e);
130+
}
131+
}
132+
};
133+
}
134+
135+
public Decoder createDecoder() {
136+
return new Decoder() {
137+
@Override
138+
public Object decode(Response response, Type type) throws IOException, DecodeException {
139+
if (response.body() == null) {
140+
logger.debug("Response body is null, returning null");
141+
return null;
142+
}
143+
String json = null;
144+
try (InputStream bodyStream = response.body().asInputStream()) {
145+
json = new String(bodyStream.readAllBytes(), StandardCharsets.UTF_8);
146+
logger.debug("Decoding JSON response: {}", json);
147+
return objectMapper.readValue(json, objectMapper.getTypeFactory().constructType(type));
148+
} catch (IOException e) {
149+
logger.error("IOException during decoding. Status: {}, Raw body: {}", response.status(), json, e);
150+
throw new DecodeException(response.status(), "Error decoding JSON response", response.request(), e);
151+
} catch (Exception e) {
152+
logger.error("Unexpected error during decoding. Status: {}, Type: {}, Raw body: {}", response.status(), type, json, e);
153+
throw new DecodeException(response.status(), "Unexpected error during decoding", response.request(), e);
154+
}
155+
}
156+
};
157+
}
158+
}

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/lifecycle/OntapPrimaryDatastoreLifecycle.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -409,10 +409,11 @@ private boolean validateProtocolSupportAndFetchHostsIdentifier(List<HostVO> host
409409
for (HostVO host : hosts) {
410410
if (host != null) {
411411
ip = host.getStorageIpAddress() != null ? host.getStorageIpAddress().trim() : "";
412-
if (ip.isEmpty() && host.getPrivateIpAddress() != null || host.getPrivateIpAddress().trim().isEmpty()) {
413-
return false;
414-
} else {
415-
ip = ip.isEmpty() ? host.getPrivateIpAddress().trim() : ip;
412+
if (ip.isEmpty()) {
413+
if (host.getPrivateIpAddress() == null || host.getPrivateIpAddress().trim().isEmpty()) {
414+
return false;
415+
}
416+
ip = host.getPrivateIpAddress().trim();
416417
}
417418
}
418419
hostIdentifiers.add(ip);

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/listener/OntapHostListener.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,11 @@ public boolean hostConnect(long hostId, long poolId) {
8787
"Unable to establish a connection from agent to storage pool %s due to %s", pool, answer.getDetails()));
8888
}
8989

90+
if (!(answer instanceof ModifyStoragePoolAnswer)) {
91+
logger.error("Received unexpected answer type {} for storage pool {}", answer.getClass().getName(), pool.getName());
92+
throw new CloudRuntimeException("Failed to connect to storage pool. Please check agent logs for details.");
93+
}
94+
9095
ModifyStoragePoolAnswer mspAnswer = (ModifyStoragePoolAnswer) answer;
9196
StoragePoolInfo poolInfo = mspAnswer.getPoolInfo();
9297
if (poolInfo == null) {

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/service/StorageStrategy.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ public boolean connect() {
134134
return false;
135135
}
136136

137-
this.aggregates = aggrs;
138137
s_logger.info("Successfully connected to ONTAP cluster and validated ONTAP details provided");
139138
} catch (Exception e) {
140139
s_logger.error("Failed to connect to ONTAP cluster: " + e.getMessage(), e);

‎plugins/storage/volume/ontap/src/main/java/org/apache/cloudstack/storage/utils/OntapStorageUtils.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.apache.logging.log4j.Logger;
3131
import org.springframework.util.Base64Utils;
3232

33+
import java.nio.charset.StandardCharsets;
3334
import java.util.Map;
3435

3536
public class OntapStorageUtils {
@@ -40,7 +41,7 @@ public class OntapStorageUtils {
4041
private static final String AUTH_HEADER_COLON = ":";
4142

4243
public static String generateAuthHeader (String username, String password) {
43-
byte[] encodedBytes = Base64Utils.encode((username + AUTH_HEADER_COLON + password).getBytes());
44+
byte[] encodedBytes = Base64Utils.encode((username + AUTH_HEADER_COLON + password).getBytes(StandardCharsets.UTF_8));
4445
return BASIC + StringUtils.SPACE + new String(encodedBytes);
4546
}
4647

0 commit comments

Comments
 (0)