diff:
@@ -96,17 +96,47 @@ public class Applications {
}
public List<CloudFoundryApplication> all() {
+ List<Application> newCloudFoundryAppList =
+ collectPages("applications", page -> api.all(page, 5000, null, null));
+
+ // Evict Records from `cache` that are no longer in the foundation
+ List<String> availableAppIds =
+ newCloudFoundryAppList.stream().map(Application::getGuid).collect(toList());
+ serverGroupCache.asMap().keySet().stream()
+ .forEach(
+ key -> {
+ if (!availableAppIds.contains(key)) {
+ log.debug("Evicting the following SG with id '" + key + "'");
+ serverGroupCache.invalidate(key);
+ }
+ });
+
+ // if the update time doesn't match then we need to update the cache
+ // if the app is not found in the cache we need to process with `map` and update the cache
+ List<Application> appsToBeUpdated =
+ newCloudFoundryAppList.stream()
+ .filter(
+ a ->
+ Optional.ofNullable(findById(a.getGuid()))
+ .map(
+ r ->
+ !r.getUpdatedTime()
+ .equals(a.getUpdatedAt().toInstant().toEpochMilli()))
+ .orElse(true))
+ .collect(Collectors.toList());
+
List<CloudFoundryServerGroup> serverGroups =
- collectPages("applications", page -> api.all(page, null, null)).stream()
- .map(this::map)
- .collect(toList());
- serverGroupCache.invalidateAll();
+ appsToBeUpdated.stream().map(this::map).collect(toList());
serverGroups.forEach(sg -> serverGroupCache.put(sg.getId(), sg));
+ // execute health check on instances, set number of available instances and health status
+ newCloudFoundryAppList.forEach(
+ a -> serverGroupCache.put(a.getGuid(), checkHealthStatus(findById(a.getGuid()), a)));
+
Map<String, Set<CloudFoundryServerGroup>> serverGroupsByClusters = new HashMap<>();
Map<String, Set<String>> clustersByApps = new HashMap<>();
- for (CloudFoundryServerGroup serverGroup : serverGroups) {
+ for (CloudFoundryServerGroup serverGroup : serverGroupCache.asMap().values()) {
Names names = Names.parseName(serverGroup.getName());
serverGroupsByClusters
.computeIfAbsent(names.getCluster(), clusterName -> new HashSet<>())
@@ -153,7 +183,7 @@ public class Applications {
.map(CloudFoundryServerGroup::getId)
.orElseGet(
() ->
- safelyCall(() -> api.all(null, singletonList(name), singletonList(spaceId)))
+ safelyCall(() -> api.all(null, 1, singletonList(name), singletonList(spaceId)))
.flatMap(
page ->
page.getResources().stream()
@@ -180,68 +210,6 @@ public class Applications {
safelyCall(() -> api.findApplicationEnvById(appId)).orElse(null);
Process process = safelyCall(() -> api.findProcessById(appId)).orElse(null);
- Set<CloudFoundryInstance> instances;
- switch (state) {
- case STOPPED:
- instances = emptySet();
- break;
- case STARTED:
- try {
- instances =
- safelyCall(() -> api.instances(appId)).orElse(emptyMap()).entrySet().stream()
- .map(
- inst -> {
- HealthState healthState = HealthState.Unknown;
- switch (inst.getValue().getState()) {
- case RUNNING:
- healthState = HealthState.Up;
- break;
- case DOWN:
- case CRASHED:
- healthState = HealthState.Down;
- break;
- case STARTING:
- healthState = HealthState.Starting;
- break;
- }
- return CloudFoundryInstance.builder()
- .appGuid(appId)
- .key(inst.getKey())
- .healthState(healthState)
- .details(inst.getValue().getDetails())
- .launchTime(
- System.currentTimeMillis() - (inst.getValue().getUptime() * 1000))
- .zone(space == null ? "unknown" : space.getName())
- .build();
- })
- .collect(toSet());
-
- log.debug(
- "Successfully retrieved "
- + instances.size()
- + " instances for application '"
- + application.getName()
- + "'");
- } catch (RetrofitError e) {
- try {
- log.debug(
- "Unable to retrieve instances for application '"
- + application.getName()
- + "': "
- + IOUtils.toString(e.getResponse().getBody().in(), Charset.defaultCharset()));
- } catch (IOException e1) {
- log.debug("Unable to retrieve droplet for application '" + application.getName() + "'");
- }
- instances = emptySet();
- } catch (Exception ex) {
- log.debug("Unable to retrieve droplet for application '" + application.getName() + "'");
- instances = emptySet();
- }
- break;
- default:
- instances = emptySet();
- }
-
CloudFoundryDroplet droplet = null;
try {
CloudFoundryPackage cfPackage =
@@ -350,7 +318,8 @@ public class Applications {
serverGroupMetricsUri = metricsUri + "/apps/" + appId;
}
- return CloudFoundryServerGroup.builder()
+ CloudFoundryServerGroup cloudFoundryServerGroup =
+ CloudFoundryServerGroup.builder()
.account(account)
.appsManagerUri(serverGroupAppManagerUri)
.metricsUri(serverGroupMetricsUri)
@@ -365,13 +334,82 @@ public class Applications {
.space(space)
.createdTime(application.getCreatedAt().toInstant().toEpochMilli())
.serviceInstances(cloudFoundryServices)
- .instances(instances)
.state(state)
.env(environmentVars)
.ciBuild(buildInfo)
.appArtifact(artifactInfo)
.pipelineId(pipelineId)
+ .updatedTime(application.getUpdatedAt().toInstant().toEpochMilli())
.build();
+
+ return checkHealthStatus(cloudFoundryServerGroup, application);
+ }
+
+ private CloudFoundryServerGroup checkHealthStatus(
+ CloudFoundryServerGroup cloudFoundryServerGroup, Application application) {
+ CloudFoundryServerGroup.State state =
+ CloudFoundryServerGroup.State.valueOf(application.getState());
+ Set<CloudFoundryInstance> instances;
+ switch (state) {
+ case STARTED:
+ try {
+ instances =
+ safelyCall(() -> api.instances(cloudFoundryServerGroup.getId())).orElse(emptyMap())
+ .entrySet().stream()
+ .map(
+ inst -> {
+ HealthState healthState = HealthState.Unknown;
+ switch (inst.getValue().getState()) {
+ case RUNNING:
+ healthState = HealthState.Up;
+ break;
+ case DOWN:
+ case CRASHED:
+ healthState = HealthState.Down;
+ break;
+ case STARTING:
+ healthState = HealthState.Starting;
+ break;
+ }
+ return CloudFoundryInstance.builder()
+ .appGuid(cloudFoundryServerGroup.getId())
+ .key(inst.getKey())
+ .healthState(healthState)
+ .details(inst.getValue().getDetails())
+ .launchTime(
+ System.currentTimeMillis() - (inst.getValue().getUptime() * 1000))
+ .zone(cloudFoundryServerGroup.getRegion())
+ .build();
+ })
+ .collect(toSet());
+
+ log.debug(
+ "Successfully retrieved "
+ + instances.size()
+ + " instances for application '"
+ + application.getName()
+ + "'");
+ } catch (RetrofitError e) {
+ try {
+ log.debug(
+ "Unable to retrieve instances for application '"
+ + application.getName()
+ + "': "
+ + IOUtils.toString(e.getResponse().getBody().in(), Charset.defaultCharset()));
+ } catch (IOException e1) {
+ log.debug("Unable to retrieve droplet for application '" + application.getName() + "'");
+ }
+ instances = emptySet();
+ } catch (Exception ex) {
+ log.debug("Unable to retrieve droplet for application '" + application.getName() + "'");
+ instances = emptySet();
+ }
+ break;
+ case STOPPED:
+ default:
+ instances = emptySet();
+ }
+ return cloudFoundryServerGroup.toBuilder().state(state).instances(instances).build();
}
private String getEnvironmentVar(

diff:
@@ -59,7 +59,7 @@ public class Routes {
private LoadingCache<String, List<RouteMapping>> routeMappings =
CacheBuilder.newBuilder()
- .expireAfterWrite(1, TimeUnit.SECONDS)
+ .expireAfterWrite(30, TimeUnit.SECONDS)
.build(
new CacheLoader<String, List<RouteMapping>>() {
@Override
@@ -111,7 +111,7 @@ public class Routes {
if (routeId.getPath() != null) queryParams.add("path:" + routeId.getPath());
if (routeId.getPort() != null) queryParams.add("port:" + routeId.getPort().toString());
- return collectPageResources("route mappings", pg -> api.all(pg, queryParams)).stream()
+ return collectPageResources("route mappings", pg -> api.all(pg, 5000, queryParams)).stream()
.filter(
routeResource ->
(routeId.getPath() != null || routeResource.getEntity().getPath().isEmpty())
@@ -142,7 +142,8 @@ public class Routes {
}
public List<CloudFoundryLoadBalancer> all() throws CloudFoundryApiException {
- List<Resource<Route>> routeResources = collectPageResources("routes", pg -> api.all(pg, null));
+ List<Resource<Route>> routeResources =
+ collectPageResources("routes", pg -> api.all(pg, 5000, null));
List<CloudFoundryLoadBalancer> loadBalancers = new ArrayList<>(routeResources.size());
for (Resource<Route> routeResource : routeResources) {
loadBalancers.add(map(routeResource));

diff:
@@ -33,6 +33,7 @@ public interface ApplicationService {
@GET("/v3/apps")
Pagination<Application> all(
@Query("page") Integer page,
+ @Query("per_page") Integer perPage,
@Query("names") List<String> names,
@Query("space_guids") List<String> spaceGuids);

diff:
@@ -28,7 +28,10 @@ public interface RouteService {
// Mapping to CF API style query params -
// https://apidocs.cloudfoundry.org/1.34.0/routes/list_all_routes.html
@GET("/v2/routes?results-per-page=100")
- Page<Route> all(@Query("page") Integer page, @Query("q") List<String> queryParams);
+ Page<Route> all(
+ @Query("page") Integer page,
+ @Query("per_page") Integer perPage,
+ @Query("q") List<String> queryParams);
@GET("/v2/routes/{guid}")
Resource<Route> findById(@Path("guid") String guid);

diff:
@@ -26,5 +26,6 @@ public class Application {
private String guid;
private String state;
private ZonedDateTime createdAt;
+ private ZonedDateTime updatedAt;
private Map<String, Link> links;
}

diff:
@@ -43,7 +43,7 @@ import lombok.experimental.Wither;
@Value
@EqualsAndHashCode(of = "id", callSuper = false)
-@Builder
+@Builder(toBuilder = true)
@JsonDeserialize(builder = CloudFoundryServerGroup.CloudFoundryServerGroupBuilder.class)
@JsonInclude(JsonInclude.Include.NON_EMPTY)
@JsonIgnoreProperties("loadBalancerNames")
@@ -89,6 +89,9 @@ public class CloudFoundryServerGroup extends CloudFoundryModel implements Server
@JsonView(Views.Cache.class)
CloudFoundrySpace space;
+ @JsonView(Views.Cache.class)
+ Long updatedTime;
+
@JsonView(Views.Cache.class)
Long createdTime;

diff:
@@ -97,6 +97,7 @@ class ApplicationsTest {
Application application =
new Application()
.setCreatedAt(ZonedDateTime.now())
+ .setUpdatedAt(ZonedDateTime.now())
.setGuid(serverGroupId)
.setName(serverGroupName)
.setState("STARTED")
@@ -321,6 +322,7 @@ class ApplicationsTest {
Application application =
new Application()
.setCreatedAt(ZonedDateTime.now())
+ .setUpdatedAt(ZonedDateTime.now())
.setGuid(expectedServerGroupId)
.setName("app")
.setState("STARTED")
@@ -331,7 +333,7 @@ class ApplicationsTest {
new Pagination<Application>()
.setPagination(new Pagination.Details().setTotalPages(1))
.setResources(Collections.singletonList(application));
- when(applicationService.all(any(), any(), any())).thenReturn(applicationPagination);
+ when(applicationService.all(any(), any(), any(), any())).thenReturn(applicationPagination);
mockMap(cloudFoundrySpace, "droplet-id");
String serverGroupId = apps.findServerGroupId(serverGroupName, spaceId);
@@ -346,6 +348,7 @@ class ApplicationsTest {
Application application =
new Application()
.setCreatedAt(ZonedDateTime.now())
+ .setUpdatedAt(ZonedDateTime.now())
.setGuid(serverGroupId)
.setName(serverGroupName)
.setState("STARTED")
@@ -364,7 +367,7 @@ class ApplicationsTest {
.setName("service-instance");
String dropletId = "droplet-guid";
- when(applicationService.all(any(), any(), any())).thenReturn(applicationPagination);
+ when(applicationService.all(any(), any(), any(), any())).thenReturn(applicationPagination);
mockMap(cloudFoundrySpace, dropletId);
CloudFoundryDroplet expectedDroplet = CloudFoundryDroplet.builder().id(dropletId).build();
@@ -379,6 +382,7 @@ class ApplicationsTest {
.instances(Collections.emptySet())
.serviceInstances(Collections.emptyList())
.createdTime(application.getCreatedAt().toInstant().toEpochMilli())
+ .updatedTime(application.getUpdatedAt().toInstant().toEpochMilli())
.memory(0)
.diskQuota(0)
.name(serverGroupName)

diff:
@@ -41,11 +41,11 @@ class CloudFoundryClientUtilsTest {
pageOne.setPagination(new Pagination.Details().setTotalPages(1));
pageOne.setResources(pageOneResources);
- when(applicationService.all(null, null, null)).thenReturn(pageOne);
+ when(applicationService.all(null, null, null, null)).thenReturn(pageOne);
List results =
CloudFoundryClientUtils.collectPages(
- "applications", page -> applicationService.all(page, null, null));
+ "applications", page -> applicationService.all(page, null, null, null));
assertThat(results).containsExactly(applicationOne);
}
@@ -64,12 +64,12 @@ class CloudFoundryClientUtilsTest {
pageTwo.setPagination(new Pagination.Details().setTotalPages(2));
pageTwo.setResources(pageTwoResources);
- when(applicationService.all(null, null, null)).thenReturn(pageOne);
- when(applicationService.all(2, null, null)).thenReturn(pageTwo);
+ when(applicationService.all(null, null, null, null)).thenReturn(pageOne);
+ when(applicationService.all(2, null, null, null)).thenReturn(pageTwo);
List results =
CloudFoundryClientUtils.collectPages(
- "applications", page -> applicationService.all(page, null, null));
+ "applications", page -> applicationService.all(page, null, null, null));
assertThat(results).containsExactly(applicationOne, applicationTwo);
}

diff:
@@ -54,7 +54,7 @@ class RoutesTest {
route.setPath("/path");
RouteService routeService = mock(RouteService.class);
- when(routeService.all(any(), any())).thenReturn(Page.singleton(route, "abc123"));
+ when(routeService.all(any(), any(), any())).thenReturn(Page.singleton(route, "abc123"));
when(routeService.routeMappings(any(), any())).thenReturn(new Page<>());
Routes routes = new Routes("pws", routeService, null, domains, spaces);
@@ -124,7 +124,7 @@ class RoutesTest {
routeMappingPage.setTotalPages(1);
when(spaces.findById("space-guid")).thenReturn(space);
- when(routeService.all(any(), any())).thenReturn(routePage);
+ when(routeService.all(any(), any(), any())).thenReturn(routePage);
when(routeService.routeMappings(any(), any())).thenReturn(routeMappingPage);
Routes routes = new Routes("pws", routeService, null, domains, spaces);

original_message: fix(cf): reduce unnecessary calls to foundation to improve performance. (#4130)


change_type: MODIFY

diff:
@@ -35,6 +35,7 @@ import com.netflix.spinnaker.clouddriver.kubernetes.KubernetesCloudProvider;
import com.netflix.spinnaker.clouddriver.kubernetes.config.CustomKubernetesResource;
import com.netflix.spinnaker.clouddriver.kubernetes.config.KubernetesCachingPolicy;
import com.netflix.spinnaker.clouddriver.kubernetes.config.KubernetesConfigurationProperties;
+import com.netflix.spinnaker.clouddriver.kubernetes.config.LinkedDockerRegistryConfiguration;
import com.netflix.spinnaker.clouddriver.kubernetes.security.KubeconfigFileHasher;
import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesCredentialFactory;
import com.netflix.spinnaker.clouddriver.kubernetes.security.KubernetesCredentials;
@@ -400,6 +401,11 @@ public class KubernetesV2Credentials implements KubernetesCredentials {
return kindMap;
}
+ @Override
+ public List<LinkedDockerRegistryConfiguration> getDockerRegistries() {
+ return Collections.emptyList();
+ }
+
public KubernetesManifest get(KubernetesKind kind, String namespace, String name) {
return runAndRecordMetrics(
"get", kind, namespace, () -> jobExecutor.get(this, kind, namespace, name));

diff:
package com.netflix.spinnaker.clouddriver.kubernetes.security;
+import com.netflix.spinnaker.clouddriver.kubernetes.config.LinkedDockerRegistryConfiguration;
import java.util.List;
import java.util.Map;
@@ -24,4 +25,6 @@ public interface KubernetesCredentials {
List<String> getDeclaredNamespaces();
Map<String, String> getSpinnakerKindMap();
+
+ List<LinkedDockerRegistryConfiguration> getDockerRegistries();
}

diff:
@@ -19,6 +19,7 @@ package com.netflix.spinnaker.clouddriver.kubernetes.security;
import static lombok.EqualsAndHashCode.Include;
import com.netflix.spinnaker.clouddriver.kubernetes.config.KubernetesConfigurationProperties.ManagedAccount;
+import com.netflix.spinnaker.clouddriver.kubernetes.config.LinkedDockerRegistryConfiguration;
import com.netflix.spinnaker.clouddriver.security.AccountCredentials;
import com.netflix.spinnaker.clouddriver.security.ProviderVersion;
import com.netflix.spinnaker.fiat.model.resources.Permissions;
@@ -87,4 +88,8 @@ public class KubernetesNamedAccountCredentials<C extends KubernetesCredentials>
public Map<String, String> getSpinnakerKindMap() {
return credentials.getSpinnakerKindMap();
}
+
+ public List<LinkedDockerRegistryConfiguration> getDockerRegistries() {
+ return credentials.getDockerRegistries();
+ }
}

license: Apache License 2.0

repo: spinnaker/clouddriver

original_message: fix(kubernetes): surface dockerRegistries to credentials endpoint (#4134)


diff:
@@ -109,8 +109,8 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
};
private static class RetryableApiException extends RuntimeException {
- RetryableApiException() {
- super();
+ RetryableApiException(String message) {
+ super(message);
}
RetryableApiException(String message, Throwable cause) {
@@ -158,13 +158,19 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
case 504:
// after retries fail, the response body for these status codes will get wrapped up
// into a CloudFoundryApiException
- throw new RetryableApiException();
+ throw new RetryableApiException(
+ "Response Code "
+ + response.code()
+ + ": "
+ + chain.request().httpUrl()
+ + " attempting retry");
}
return response;
});
} catch (SocketTimeoutException e) {
- throw new RetryableApiException("Timeout " + callName, e);
+ throw new RetryableApiException(
+ "Timeout " + callName + " " + chain.request().httpUrl() + ", attempting retrying", e);
} catch (Exception e) {
final Response response = lastResponse.get();
if (response == null) {

original_message: feat(cf): provide more info when retrying an API (#4137)


diff:
@@ -25,18 +25,24 @@ import java.util.Optional;
import java.util.stream.Collectors;
import javax.annotation.Nullable;
import lombok.Getter;
+import retrofit.RetrofitError;
@Getter
public class CloudFoundryApiException extends RuntimeException {
- private static final String UNKNOWN_ERROR = "Unknown Error";
@Nullable private ErrorDescription.Code errorCode;
- public CloudFoundryApiException(ErrorDescription errorCause) {
+ public CloudFoundryApiException(ErrorDescription errorCause, RetrofitError retrofitError) {
super(
Optional.ofNullable(errorCause)
.map(e -> getMessage(e.getErrors().toArray(new String[0])))
- .orElse(UNKNOWN_ERROR));
+ .orElse(
+ "status: "
+ + retrofitError.getResponse().getStatus()
+ + ". url: "
+ + retrofitError.getResponse().getUrl()
+ + ". raw response body: "
+ + retrofitError.getResponse().getBody()));
if (errorCause != null) {
this.errorCode = errorCause.getCode();
}

diff:
@@ -33,7 +33,7 @@ final class CloudFoundryClientUtils {
} catch (RetrofitError retrofitError) {
if (retrofitError.getResponse() == null || retrofitError.getResponse().getStatus() != 404) {
throw new CloudFoundryApiException(
- (ErrorDescription) retrofitError.getBodyAs(ErrorDescription.class));
+ (ErrorDescription) retrofitError.getBodyAs(ErrorDescription.class), retrofitError);
}
}
}
@@ -52,7 +52,7 @@ final class CloudFoundryClientUtils {
if (errorDescription == null) {
throw new CloudFoundryApiException(retrofitError.getCause());
}
- throw new CloudFoundryApiException(errorDescription);
+ throw new CloudFoundryApiException(errorDescription, retrofitError);
}
}
}

diff:
@@ -21,17 +21,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.PropertyNamingStrategy;
import com.fasterxml.jackson.databind.SerializationFeature;
import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.ApplicationService;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.AuthenticationService;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.ConfigService;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.DomainService;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.DopplerService;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.OrganizationService;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.RouteService;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.ServiceInstanceService;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.ServiceKeyService;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.SpaceService;
-import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.TaskService;
+import com.netflix.spinnaker.clouddriver.cloudfoundry.client.api.*;
import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.Token;
import com.squareup.okhttp.Interceptor;
import com.squareup.okhttp.OkHttpClient;
@@ -63,6 +53,8 @@ import okio.Buffer;
import okio.BufferedSource;
import org.apache.commons.fileupload.MultipartStream;
import org.cloudfoundry.dropsonde.events.EventFactory.Envelope;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import retrofit.RequestInterceptor;
import retrofit.RestAdapter;
import retrofit.client.OkClient;
@@ -99,6 +91,8 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
private Tasks tasks;
private Logs logs;
+ Logger logger = LoggerFactory.getLogger(HttpCloudFoundryClient.class);
+
private final RequestInterceptor oauthInterceptor =
new RequestInterceptor() {
@Override
@@ -127,7 +121,7 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
.intervalFunction(IntervalFunction.ofExponentialBackoff(Duration.ofSeconds(10), 3))
.retryExceptions(RetryableApiException.class)
.build());
-
+ logger.debug("cf request: " + chain.request().urlString());
AtomicReference<Response> lastResponse = new AtomicReference<>();
try {
return retry.executeCallable(

diff:
package com.netflix.spinnaker.clouddriver.cloudfoundry.client;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.*;
import com.netflix.spinnaker.clouddriver.cloudfoundry.client.model.ErrorDescription;
import java.util.Arrays;
+import java.util.Collections;
import org.junit.jupiter.api.Test;
+import retrofit.RetrofitError;
+import retrofit.client.Response;
class CloudFoundryApiExceptionTest {
@Test
void constructorWithErrorDescription() {
+ Response response = new Response("url", 500, "reason", Collections.emptyList(), null);
+ RetrofitError retrofitError = mock(RetrofitError.class);
+ when(retrofitError.getResponse()).thenReturn(response);
CloudFoundryApiException e =
new CloudFoundryApiException(
new ErrorDescription()
.setErrors(
Arrays.asList(
new ErrorDescription.Detail().setDetail("Main Error"),
- new ErrorDescription.Detail().setDetail("Foo"))));
+ new ErrorDescription.Detail().setDetail("Foo"))),
+ retrofitError);
assertThat(e.getMessage()).contains("Main Error and Foo");
}
@Test
void constructorHandlesNullErrorDescription() {
- CloudFoundryApiException e = new CloudFoundryApiException((ErrorDescription) null);
+ Response response = new Response("url", 500, "reason", Collections.emptyList(), null);
+ RetrofitError retrofitError = mock(RetrofitError.class);
+ when(retrofitError.getResponse()).thenReturn(response);
+ CloudFoundryApiException e =
+ new CloudFoundryApiException((ErrorDescription) null, retrofitError);
- assertThat(e.getMessage()).contains("Unknown Error");
+ assertThat(e.getMessage()).contains("status: 500. url: url. raw response body: null");
}
}

original_message: feat(cf): increase logging for cf to better detect fails (#4156)


diff:
@@ -75,7 +75,7 @@ public class Keys implements KeyParser {
}
public static String getApplicationKey(String app) {
- return ID + ":" + Namespace.APPLICATIONS + ":" + app;
+ return ID + ":" + Namespace.APPLICATIONS + ":" + app.toLowerCase();
}
public static String getAllLoadBalancers() {
@@ -132,11 +132,27 @@ public class Keys implements KeyParser {
}
public static String getClusterKey(String account, String app, String name) {
- return ID + ":" + Namespace.CLUSTERS + ":" + account + ":" + app + ":" + name;
+ return ID
+ + ":"
+ + Namespace.CLUSTERS
+ + ":"
+ + account
+ + ":"
+ + app.toLowerCase()
+ + ":"
+ + name.toLowerCase();
}
public static String getServerGroupKey(String account, String name, String region) {
- return ID + ":" + Namespace.SERVER_GROUPS + ":" + account + ":" + name + ":" + region;
+ return ID
+ + ":"
+ + Namespace.SERVER_GROUPS
+ + ":"
+ + account
+ + ":"
+ + name.toLowerCase()
+ + ":"
+ + region;
}
public static String getInstanceKey(String account, String instanceName) {

diff:
@@ -102,7 +102,9 @@ public class Applications {
// Evict Records from `cache` that are no longer in the foundation
List<String> availableAppIds =
newCloudFoundryAppList.stream().map(Application::getGuid).collect(toList());
- serverGroupCache.asMap().keySet().stream()
+ serverGroupCache
+ .asMap()
+ .keySet()
.forEach(
key -> {
if (!availableAppIds.contains(key)) {
@@ -168,7 +170,7 @@ public class Applications {
@Nullable
public CloudFoundryServerGroup findServerGroupByNameAndSpaceId(String name, String spaceId) {
return Optional.ofNullable(findServerGroupId(name, spaceId))
- .map(serverGroupId -> Optional.ofNullable(findById(serverGroupId)).orElse(null))
+ .flatMap(serverGroupId -> Optional.ofNullable(findById(serverGroupId)))
.orElse(null);
}
@@ -214,7 +216,7 @@ public class Applications {
try {
CloudFoundryPackage cfPackage =
safelyCall(() -> api.findPackagesByAppId(appId))
- .map(
+ .flatMap(
packages ->
packages.getResources().stream()
.findFirst()
@@ -233,8 +235,7 @@ public class Applications {
pkg.getData().getChecksum() == null
? null
: pkg.getData().getChecksum().getValue())
- .build())
- .orElse(null))
+ .build()))
.orElse(null);
droplet =
@@ -304,13 +305,17 @@ public class Applications {
String serverGroupAppManagerUri = appsManagerUri;
if (StringUtils.isNotEmpty(appsManagerUri)) {
serverGroupAppManagerUri =
+ Optional.ofNullable(space)
+ .map(
+ s ->
appsManagerUri
+ "/organizations/"
- + space.getOrganization().getId()
+ + s.getOrganization().getId()
+ "/spaces/"
- + space.getId()
+ + s.getId()
+ "/applications/"
- + appId;
+ + appId)
+ .orElse("");
}
String serverGroupMetricsUri = metricsUri;
@@ -504,7 +509,7 @@ public class Applications {
throws CloudFoundryApiException {
final Process.HealthCheck healthCheck =
healthCheckType != null ? new Process.HealthCheck().setType(healthCheckType) : null;
- if (healthCheckEndpoint != null && !healthCheckEndpoint.isEmpty()) {
+ if (healthCheckEndpoint != null && !healthCheckEndpoint.isEmpty() && healthCheck != null) {
healthCheck.setData(new Process.HealthCheckData().setEndpoint(healthCheckEndpoint));
}
safelyCall(() -> api.updateProcess(guid, new UpdateProcess(command, healthCheck)));
@@ -539,9 +544,9 @@ public class Applications {
}
}
- public String uploadPackageBits(String packageGuid, File file) throws CloudFoundryApiException {
+ public void uploadPackageBits(String packageGuid, File file) throws CloudFoundryApiException {
TypedFile uploadFile = new TypedFile("multipart/form-data", file);
- return safelyCall(() -> api.uploadPackageBits(packageGuid, uploadFile))
+ safelyCall(() -> api.uploadPackageBits(packageGuid, uploadFile))
.map(Package::getGuid)
.orElseThrow(
() ->

diff:
@@ -36,18 +36,16 @@ public class CloudFoundryApiException extends RuntimeException {
super(
Optional.ofNullable(errorCause)
.map(e -> getMessage(e.getErrors().toArray(new String[0])))
- .orElse(
- "status: "
- + retrofitError.getResponse().getStatus()
- + ". url: "
- + retrofitError.getResponse().getUrl()
- + ". raw response body: "
- + retrofitError.getResponse().getBody()));
+ .orElse(getRetrofitErrorMessage(retrofitError)));
if (errorCause != null) {
this.errorCode = errorCause.getCode();
}
}
+ public CloudFoundryApiException(RetrofitError retrofitError) {
+ super(getRetrofitErrorMessage(retrofitError));
+ }
+
public CloudFoundryApiException(Throwable t, String... errors) {
super(getMessage(t, errors), t);
}
@@ -66,4 +64,13 @@ public class CloudFoundryApiException extends RuntimeException {
allErrors[errors.length] = t.getMessage();
return getMessage(allErrors);
}
+
+ private static String getRetrofitErrorMessage(RetrofitError retrofitError) {
+ return "status: "
+ + retrofitError.getResponse().getStatus()
+ + ". url: "
+ + retrofitError.getResponse().getUrl()
+ + ". raw response body: "
+ + retrofitError.getResponse().getBody();
+ }
}

diff:
@@ -50,7 +50,7 @@ final class CloudFoundryClientUtils {
ErrorDescription errorDescription =
(ErrorDescription) retrofitError.getBodyAs(ErrorDescription.class);
if (errorDescription == null) {
- throw new CloudFoundryApiException(retrofitError.getCause());
+ throw new CloudFoundryApiException(retrofitError);
}
throw new CloudFoundryApiException(errorDescription, retrofitError);
}

diff:
@@ -74,6 +74,7 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
private final String user;
private final String password;
private final OkHttpClient okHttpClient;
+ private Logger logger = LoggerFactory.getLogger(HttpCloudFoundryClient.class);
private AuthenticationService uaaService;
private AtomicLong tokenExpirationNs = new AtomicLong(System.nanoTime());
@@ -91,8 +92,6 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
private Tasks tasks;
private Logs logs;
- Logger logger = LoggerFactory.getLogger(HttpCloudFoundryClient.class);
-
private final RequestInterceptor oauthInterceptor =
new RequestInterceptor() {
@Override
@@ -164,7 +163,7 @@ public class HttpCloudFoundryClient implements CloudFoundryClient {
});
} catch (SocketTimeoutException e) {
throw new RetryableApiException(
- "Timeout " + callName + " " + chain.request().httpUrl() + ", attempting retrying", e);
+ "Timeout " + callName + " " + chain.request().httpUrl() + ", attempting retry", e);
} catch (Exception e) {
final Response response = lastResponse.get();
if (response == null) {

diff:
@@ -111,7 +111,7 @@ public class DeployCloudFoundryServerGroupAtomicOperation
return deploymentResult();
}
- final Integer desiredInstanceCount = description.getApplicationAttributes().getInstances();
+ final int desiredInstanceCount = description.getApplicationAttributes().getInstances();
if (description.isStartApplication() && desiredInstanceCount > 0) {
client.getApplications().startApplication(serverGroup.getId());
ProcessStats.State state =
@@ -285,7 +285,6 @@ public class DeployCloudFoundryServerGroupAtomicOperation
private static ExternalReference resolveBuildInfo(
DeployCloudFoundryServerGroupDescription description) {
Map<String, Object> buildInfo = null;
- if (buildInfo == null) {
final Artifact applicationArtifact = description.getApplicationArtifact();
if (applicationArtifact != null) {
final Map<String, Object> metadata = applicationArtifact.getMetadata();
@@ -293,7 +292,6 @@ public class DeployCloudFoundryServerGroupAtomicOperation
buildInfo = (Map<String, Object>) applicationArtifact.getMetadata().get("build");
}
}
- }
if (buildInfo == null) {
final Map<String, Object> trigger = description.getTrigger();
if (trigger != null) {

original_message: refactor(cf): removed unused code, fix spelling and etc (#4159)


diff:
@@ -66,11 +66,16 @@ public class CloudFoundryApiException extends RuntimeException {
}
private static String getRetrofitErrorMessage(RetrofitError retrofitError) {
+ return Optional.ofNullable(retrofitError.getResponse())
+ .map(
+ response -> {
return "status: "
- + retrofitError.getResponse().getStatus()
+ + response.getStatus()
+ ". url: "
- + retrofitError.getResponse().getUrl()
+ + response.getUrl()
+ ". raw response body: "
- + retrofitError.getResponse().getBody();
+ + response.getBody();
+ })
+ .orElse("error, url :" + retrofitError.getUrl() + " body: " + retrofitError.getBody());
}
}

diff:
package com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.converters;
-import com.netflix.spinnaker.clouddriver.artifacts.ArtifactDownloader;
import com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryClient;
import com.netflix.spinnaker.clouddriver.cloudfoundry.model.CloudFoundrySpace;
import com.netflix.spinnaker.clouddriver.cloudfoundry.security.CloudFoundryCredentials;
import com.netflix.spinnaker.clouddriver.security.AbstractAtomicOperationsCredentialsSupport;
-import com.netflix.spinnaker.kork.artifacts.model.Artifact;
-import java.io.IOException;
-import java.io.InputStream;
-import java.io.UncheckedIOException;
import java.util.Map;
import java.util.Optional;
-import java.util.function.Consumer;
-import org.yaml.snakeyaml.Yaml;
-import org.yaml.snakeyaml.constructor.SafeConstructor;
public abstract class AbstractCloudFoundryAtomicOperationConverter
extends AbstractAtomicOperationsCredentialsSupport {
@@ -42,15 +34,4 @@ public abstract class AbstractCloudFoundryAtomicOperationConverter
CloudFoundryCredentials credentials = getCredentialsObject(input.get("credentials").toString());
return credentials.getClient();
}
-
- void downloadAndProcessManifest(
- ArtifactDownloader downloader, Artifact manifest, Consumer<Map> processManifest) {
- try {
- InputStream manifestInput = downloader.download(manifest);
- Yaml parser = new Yaml(new SafeConstructor());
- processManifest.accept(parser.load(manifestInput));
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- }
}

diff:
package com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.converters;
import com.netflix.spinnaker.clouddriver.artifacts.ArtifactCredentialsRepository;
-import com.netflix.spinnaker.clouddriver.artifacts.ArtifactDownloader;
import com.netflix.spinnaker.clouddriver.cloudfoundry.CloudFoundryOperation;
import com.netflix.spinnaker.clouddriver.helpers.OperationPoller;
import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperations;
@@ -30,8 +29,7 @@ public class CloneCloudFoundryServerGroupAtomicOperationConverter
extends DeployCloudFoundryServerGroupAtomicOperationConverter {
public CloneCloudFoundryServerGroupAtomicOperationConverter(
@Qualifier("cloudFoundryOperationPoller") OperationPoller operationPoller,
- ArtifactCredentialsRepository credentialsRepository,
- ArtifactDownloader artifactDownloader) {
- super(operationPoller, credentialsRepository, artifactDownloader);
+ ArtifactCredentialsRepository credentialsRepository) {
+ super(operationPoller, credentialsRepository);
}
}

diff:
@@ -25,7 +25,6 @@ import com.fasterxml.jackson.databind.DeserializationFeature;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.PropertyNamingStrategy;
import com.netflix.spinnaker.clouddriver.artifacts.ArtifactCredentialsRepository;
-import com.netflix.spinnaker.clouddriver.artifacts.ArtifactDownloader;
import com.netflix.spinnaker.clouddriver.artifacts.config.ArtifactCredentials;
import com.netflix.spinnaker.clouddriver.cloudfoundry.CloudFoundryOperation;
import com.netflix.spinnaker.clouddriver.cloudfoundry.artifacts.CloudFoundryArtifactCredentials;
@@ -50,15 +49,12 @@ public class DeployCloudFoundryServerGroupAtomicOperationConverter
extends AbstractCloudFoundryServerGroupAtomicOperationConverter {
private final OperationPoller operationPoller;
private final ArtifactCredentialsRepository credentialsRepository;
- private final ArtifactDownloader artifactDownloader;
public DeployCloudFoundryServerGroupAtomicOperationConverter(
@Qualifier("cloudFoundryOperationPoller") OperationPoller operationPoller,
- ArtifactCredentialsRepository credentialsRepository,
- ArtifactDownloader artifactDownloader) {
+ ArtifactCredentialsRepository credentialsRepository) {
this.operationPoller = operationPoller;
this.credentialsRepository = credentialsRepository;
- this.artifactDownloader = artifactDownloader;
}
@Override
@@ -87,11 +83,9 @@ public class DeployCloudFoundryServerGroupAtomicOperationConverter
// the deploy operation.
converted.setArtifactCredentials(getArtifactCredentials(converted));
- downloadAndProcessManifest(
- artifactDownloader,
- converted.getManifest(),
- myMap -> converted.setApplicationAttributes(convertManifest(myMap)));
-
+ converted.setApplicationAttributes(
+ convertManifest(
+ converted.getManifest().stream().findFirst().orElse(Collections.emptyMap())));
return converted;
}
@@ -116,7 +110,8 @@ public class DeployCloudFoundryServerGroupAtomicOperationConverter
}
// visible for testing
- DeployCloudFoundryServerGroupDescription.ApplicationAttributes convertManifest(Map manifestMap) {
+ DeployCloudFoundryServerGroupDescription.ApplicationAttributes convertManifest(
+ Map<Object, Object> manifestMap) {
List<CloudFoundryManifest> manifestApps =
new ObjectMapper()
.setPropertyNamingStrategy(PropertyNamingStrategy.KEBAB_CASE)

diff:
@@ -20,20 +20,16 @@ import com.fasterxml.jackson.annotation.JsonAlias;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.core.type.TypeReference;
-import com.fasterxml.jackson.databind.DeserializationContext;
-import com.fasterxml.jackson.databind.DeserializationFeature;
-import com.fasterxml.jackson.databind.JsonDeserializer;
-import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.*;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.fasterxml.jackson.dataformat.yaml.YAMLFactory;
-import com.netflix.spinnaker.clouddriver.artifacts.ArtifactDownloader;
import com.netflix.spinnaker.clouddriver.cloudfoundry.CloudFoundryOperation;
import com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.description.DeployCloudFoundryServiceDescription;
import com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.ops.DeployCloudFoundryServiceAtomicOperation;
import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperation;
import com.netflix.spinnaker.clouddriver.orchestration.AtomicOperations;
-import com.netflix.spinnaker.kork.artifacts.model.Artifact;
import java.io.IOException;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;
@@ -46,12 +42,11 @@ import org.springframework.stereotype.Component;
public class DeployCloudFoundryServiceAtomicOperationConverter
extends AbstractCloudFoundryAtomicOperationConverter {
private static final ObjectMapper objectMapper =
- new ObjectMapper().disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
- private final ArtifactDownloader artifactDownloader;
+ new ObjectMapper()
+ .setPropertyNamingStrategy(PropertyNamingStrategy.KEBAB_CASE)
+ .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
- public DeployCloudFoundryServiceAtomicOperationConverter(ArtifactDownloader artifactDownloader) {
- this.artifactDownloader = artifactDownloader;
- }
+ public DeployCloudFoundryServiceAtomicOperationConverter() {}
@Override
public AtomicOperation convertOperation(Map input) {
@@ -70,33 +65,22 @@ public class DeployCloudFoundryServiceAtomicOperationConverter
new IllegalArgumentException(
"Unable to find space '" + converted.getRegion() + "'.")));
- Map manifest = (Map) input.get("manifest");
- if (manifest.get("artifact") != null) {
- Artifact manifestArtifact =
- objectMapper.convertValue(manifest.get("artifact"), Artifact.class);
- downloadAndProcessManifest(
- artifactDownloader,
- manifestArtifact,
- myMap -> {
- if (converted.isUserProvided()) {
- converted.setUserProvidedServiceAttributes(convertUserProvidedServiceManifest(myMap));
- } else {
- converted.setServiceAttributes(convertManifest(myMap));
- }
- });
- } else if (manifest.get("direct") != null) {
+ List<Map<Object, Object>> manifest = converted.getManifest();
+
if (converted.isUserProvided()) {
converted.setUserProvidedServiceAttributes(
- convertUserProvidedServiceManifest(manifest.get("direct")));
+ convertUserProvidedServiceManifest(manifest.stream().findFirst().orElse(null)));
} else {
- converted.setServiceAttributes(convertManifest(manifest.get("direct")));
- }
+ converted.setServiceAttributes(convertManifest(manifest.stream().findFirst().orElse(null)));
}
return converted;
}
// visible for testing
DeployCloudFoundryServiceDescription.ServiceAttributes convertManifest(Object manifestMap) {
+ if (manifestMap == null) {
+ throw new IllegalArgumentException("No configurations detected");
+ }
ServiceManifest manifest = objectMapper.convertValue(manifestMap, ServiceManifest.class);
if (manifest.getService() == null) {
throw new IllegalArgumentException("Manifest is missing the service");
@@ -119,6 +103,9 @@ public class DeployCloudFoundryServiceAtomicOperationConverter
// visible for testing
DeployCloudFoundryServiceDescription.UserProvidedServiceAttributes
convertUserProvidedServiceManifest(Object manifestMap) {
+ if (manifestMap == null) {
+ throw new IllegalArgumentException("No configurations detected");
+ }
UserProvidedServiceManifest manifest =
objectMapper.convertValue(manifestMap, UserProvidedServiceManifest.class);
if (manifest.getServiceInstanceName() == null) {
@@ -140,10 +127,10 @@ public class DeployCloudFoundryServiceAtomicOperationConverter
private String service;
private boolean updatable = true;
- @JsonAlias("service_instance_name")
+ @JsonAlias({"service_instance_name", "serviceInstanceName"})
private String serviceInstanceName;
- @JsonAlias("service_plan")
+ @JsonAlias({"service_plan", "servicePlan"})
private String servicePlan;
@Nullable private Set<String> tags;
@@ -157,21 +144,21 @@ public class DeployCloudFoundryServiceAtomicOperationConverter
private static class UserProvidedServiceManifest {
private boolean updatable = true;
- @JsonAlias("service_instance_name")
+ @JsonAlias({"service_instance_name", "serviceInstanceName"})
private String serviceInstanceName;
@Nullable
- @JsonAlias("syslog_drain_url")
+ @JsonAlias({"syslog_drain_url", "syslogDrainUrl"})
private String syslogDrainUrl;
@Nullable
- @JsonAlias("route_service_url")
+ @JsonAlias({"route_service_url", "routeServiceUrl"})
private String routeServiceUrl;
@Nullable private Set<String> tags;
@Nullable
- @JsonAlias("credentials_map")
+ @JsonAlias({"credentials_map", "credentialsMap"})
@JsonDeserialize(using = OptionallySerializedMapDeserializer.class)
private Map<String, Object> credentials;
}

diff:
@@ -37,7 +37,7 @@ public class DeployCloudFoundryServerGroupDescription
private CloudFoundrySpace space;
private boolean startApplication;
private Artifact applicationArtifact;
- private Artifact manifest;
+ private List<Map<Object, Object>> manifest;
private String executionId;
private Map<String, Object> trigger;

diff:
package com.netflix.spinnaker.clouddriver.cloudfoundry.deploy.description;
import com.fasterxml.jackson.annotation.JsonIgnore;
-import com.netflix.spinnaker.clouddriver.artifacts.config.ArtifactCredentials;
-import com.netflix.spinnaker.kork.artifacts.model.Artifact;
+import java.util.List;
import java.util.Map;
import java.util.Set;
import javax.annotation.Nullable;
@@ -30,9 +29,7 @@ import lombok.EqualsAndHashCode;
public class DeployCloudFoundryServiceDescription extends AbstractCloudFoundryServiceDescription {
private boolean userProvided = false;
- @JsonIgnore private Artifact artifact;
-
- @JsonIgnore private ArtifactCredentials artifactCredentials;
+ private List<Map<Object, Object>> manifest;
@JsonIgnore private ServiceAttributes serviceAttributes;

diff:
@@ -22,7 +22,6 @@ import static org.mockito.Mockito.when;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.clouddriver.artifacts.ArtifactCredentialsRepository;
-import com.netflix.spinnaker.clouddriver.artifacts.ArtifactDownloader;
import com.netflix.spinnaker.clouddriver.cloudfoundry.artifacts.ArtifactCredentialsFromString;
import com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryClient;
import com.netflix.spinnaker.clouddriver.cloudfoundry.client.MockCloudFoundryClient;
@@ -105,9 +104,7 @@ class DeployCloudFoundryServerGroupAtomicOperationConverterTest {
private final DeployCloudFoundryServerGroupAtomicOperationConverter converter =
new DeployCloudFoundryServerGroupAtomicOperationConverter(
- null,
- artifactCredentialsRepository,
- new ArtifactDownloader(artifactCredentialsRepository));
+ null, artifactCredentialsRepository);
@BeforeEach
void initializeClassUnderTest() {

diff:
@@ -24,7 +24,6 @@ import static org.mockito.Mockito.when;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.annotation.JsonDeserialize;
import com.netflix.spinnaker.clouddriver.artifacts.ArtifactCredentialsRepository;
-import com.netflix.spinnaker.clouddriver.artifacts.ArtifactDownloader;
import com.netflix.spinnaker.clouddriver.cloudfoundry.artifacts.ArtifactCredentialsFromString;
import com.netflix.spinnaker.clouddriver.cloudfoundry.client.CloudFoundryClient;
import com.netflix.spinnaker.clouddriver.cloudfoundry.client.MockCloudFoundryClient;
@@ -104,8 +103,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
private final AccountCredentialsProvider accountCredentialsProvider =
new DefaultAccountCredentialsProvider(accountCredentialsRepository);
private final DeployCloudFoundryServiceAtomicOperationConverter converter =
- new DeployCloudFoundryServiceAtomicOperationConverter(
- new ArtifactDownloader(artifactCredentialsRepository));
+ new DeployCloudFoundryServiceAtomicOperationConverter();
@BeforeEach
void initializeClassUnderTest() {
@@ -141,7 +139,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
HashMap.of(
"service_instance_name", "my-service-instance-name",
"service_plan", "my-service-plan",
- "tags", List.of("my-tag").asJava(),
+ "tags", Collections.singletonList("my-tag"),
"parameters", "{\"foo\": \"bar\"}")
.toJavaMap();
@@ -157,7 +155,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
HashMap.of(
"service_instance_name", "my-service-instance-name",
"service_plan", "my-service-plan",
- "tags", List.of("my-tag").asJava(),
+ "tags", Collections.singletonList("my-tag"),
"parameters", "{\"foo\": \"bar\"}")
.toJavaMap();
@@ -173,7 +171,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
HashMap.of(
"service", "my-service",
"service_instance_name", "my-service-instance-name",
- "tags", List.of("my-tag").asJava(),
+ "tags", Collections.singletonList("my-tag"),
"parameters", "{\"foo\": \"bar\"}")
.toJavaMap();
@@ -191,7 +189,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
"syslog_drain_url", "test-syslog-drain-url",
"updatable", false,
"route_service_url", "test-route-service-url",
- "tags", List.of("my-tag").asJava(),
+ "tags", Collections.singletonList("my-tag"),
"credentials_map", "{\"foo\": \"bar\"}")
.toJavaMap();
@@ -203,7 +201,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
.setRouteServiceUrl("test-route-service-url")
.setTags(Collections.singleton("my-tag"))
.setUpdatable(false)
- .setCredentials(HashMap.<String, Object>of("foo", "bar").toJavaMap()));
+ .setCredentials(Collections.singletonMap("foo", "bar")));
}
@Test
@@ -212,7 +210,7 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
HashMap.of(
"syslog_drain_url", "test-syslog-drain-url",
"route_service_url", "test-route-service-url",
- "tags", List.of("my-tag").asJava(),
+ "tags", Collections.singletonList("my-tag"),
"credentials_map", "{\"foo\": \"bar\"}")
.toJavaMap();
@@ -222,43 +220,6 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
"Manifest is missing the service name");
}
- @Test
- void convertDescriptionWithDownloadedManifest() {
- final Map input =
- HashMap.of(
- "credentials", "test",
- "region", "org > space",
- "manifest",
- HashMap.of(
- "artifact",
- HashMap.of(
- "artifactAccount", "test",
- "reference", "ref1",
- "type", "test")
- .toJavaMap())
- .toJavaMap())
- .toJavaMap();
-
- final DeployCloudFoundryServiceDescription result = converter.convertDescription(input);
-
- assertThat(result.getSpace())
- .isEqualToComparingFieldByFieldRecursively(
- CloudFoundrySpace.builder()
- .id("space-guid")
- .name("space")
- .organization(CloudFoundryOrganization.builder().id("org-guid").name("org").build())
- .build());
- assertThat(result.getServiceAttributes())
- .isEqualToComparingFieldByFieldRecursively(
- new DeployCloudFoundryServiceDescription.ServiceAttributes()
- .setService("my-service")
- .setServiceInstanceName("my-service-instance-name")
- .setServicePlan("my-service-plan")
- .setTags(Collections.singleton("tag1"))
- .setUpdatable(false)
- .setParameterMap(HashMap.<String, Object>of("foo", "bar").toJavaMap()));
- }
-
@Test
void convertDescriptionWithUserProvidedInput() {
final Map input =
@@ -270,16 +231,14 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
"userProvided",
true,
"manifest",
- HashMap.of(
- "direct",
+ Collections.singletonList(
HashMap.of(
"serviceInstanceName", "userProvidedServiceName",
- "tags", List.of("my-tag").asJava(),
+ "tags", Collections.singletonList("my-tag"),
"syslogDrainUrl", "http://syslogDrainUrl.io",
"credentials", "{\"foo\": \"bar\"}",
"routeServiceUrl", "http://routeServiceUrl.io")
- .toJavaMap())
- .toJavaMap())
+ .toJavaMap()))
.toJavaMap();
final DeployCloudFoundryServiceDescription result = converter.convertDescription(input);
@@ -295,15 +254,11 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
.setCredentials(HashMap.<String, Object>of("foo", "bar").toJavaMap()));
}
- private static <K, V> Map<K, V> mapOf(K key, V value) {
- return HashMap.of(key, value).toJavaMap();
- }
-
private static class WithMap {
public WithMap() {}
public WithMap(String key, Object value) {
- this.mapField = HashMap.of(key, value).toJavaMap();
+ this.mapField = Collections.singletonMap(key, value);
}
@Nullable
@@ -317,7 +272,8 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
@Test
void deserializeYamlSerializedMap() {
final WithMap result =
- new ObjectMapper().convertValue(mapOf("mapField", "key1: value1"), WithMap.class);
+ new ObjectMapper()
+ .convertValue(Collections.singletonMap("mapField", "key1: value1"), WithMap.class);
assertThat(result).isEqualToComparingFieldByFieldRecursively(new WithMap("key1", "value1"));
}
@@ -326,7 +282,8 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
void deserializeJsonSerializedMap() {
final WithMap result =
new ObjectMapper()
- .convertValue(mapOf("mapField", "{\"key1\": \"value1\"}}"), WithMap.class);
+ .convertValue(
+ Collections.singletonMap("mapField", "{\"key1\": \"value1\"}}"), WithMap.class);
assertThat(result).isEqualToComparingFieldByFieldRecursively(new WithMap("key1", "value1"));
}
@@ -334,21 +291,26 @@ class DeployCloudFoundryServiceAtomicOperationConverterTest {
@Test
void deserializeAlreadyDeserializedMap() {
final WithMap result =
- new ObjectMapper().convertValue(mapOf("mapField", mapOf("key1", "value1")), WithMap.class);
+ new ObjectMapper()
+ .convertValue(
+ Collections.singletonMap("mapField", Collections.singletonMap("key1", "value1")),
+ WithMap.class);
assertThat(result).isEqualToComparingFieldByFieldRecursively(new WithMap("key1", "value1"));
}
@Test
void deserializeEmptyStringAsMap() {
- final WithMap result = new ObjectMapper().convertValue(mapOf("mapField", ""), WithMap.class);
+ final WithMap result =
+ new ObjectMapper().convertValue(Collections.singletonMap("mapField", ""), WithMap.class);
assertThat(result).isEqualToComparingFieldByFieldRecursively(new WithMap());
}
@Test
void deserializeNullStringAsMap() {
- final WithMap result = new ObjectMapper().convertValue(mapOf("mapField", null), WithMap.class);
+ final WithMap result =
+ new ObjectMapper().convertValue(Collections.singletonMap("mapField", null), WithMap.class);
assertThat(result).isEqualToComparingFieldByFieldRecursively(new WithMap());
}

original_message: feat(cf): add SpEL support for cf manifests (#4167)


diff:
@@ -115,6 +115,9 @@ public class KubernetesV2SecurityGroup extends ManifestBasedModel implements Sec
}
private static Set<Rule> inboundRules(V1NetworkPolicy policy) {
+ if (policy.getSpec().getIngress() == null) {
+ return Collections.emptySet();
+ }
return policy.getSpec().getIngress().stream()
.map(i -> i.getPorts().stream().map(KubernetesV2SecurityGroup::fromPolicyPort))
.flatMap(s -> s)
@@ -122,6 +125,9 @@ public class KubernetesV2SecurityGroup extends ManifestBasedModel implements Sec
}
private static Set<Rule> outboundRules(V1NetworkPolicy policy) {
+ if (policy.getSpec().getEgress() == null) {
+ return Collections.emptySet();
+ }
return policy.getSpec().getEgress().stream()
.map(i -> i.getPorts().stream().map(KubernetesV2SecurityGroup::fromPolicyPort))
.flatMap(s -> s)

original_message: fix(kubernetes): do not throw NPE on NetworkPolicies with null ingress and egress (#4172)


diff:
@@ -42,7 +42,6 @@ import lombok.Builder;
import lombok.NoArgsConstructor;
import lombok.Value;
import lombok.experimental.NonFinal;
-import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
@@ -106,9 +105,9 @@ public class LoadFront50App implements SagaAction<LoadFront50App.LoadFront50AppC
return command;
}
- @NotNull
+ @Nonnull
@Override
- public Result apply(@NotNull LoadFront50AppCommand command, @NotNull Saga saga) {
+ public Result apply(@Nonnull LoadFront50AppCommand command, @Nonnull Saga saga) {
try {
Map response = front50Service.getApplication(command.getAppName());
try {
@@ -154,7 +153,7 @@ public class LoadFront50App implements SagaAction<LoadFront50App.LoadFront50AppC
this.metadata = metadata;
}
- @NotNull
+ @Nonnull
@Override
public List<SpinnakerEvent> getComposedEvents() {
return Collections.singletonList(nextCommand);

diff:
@@ -30,7 +30,6 @@ import javax.annotation.Nullable;
import lombok.Builder;
import lombok.Value;
import lombok.experimental.NonFinal;
-import org.jetbrains.annotations.NotNull;
import org.springframework.stereotype.Component;
/**
@@ -41,9 +40,9 @@ import org.springframework.stereotype.Component;
public class SnapshotAtomicOperationInput
implements SagaAction<SnapshotAtomicOperationInput.SnapshotAtomicOperationInputCommand> {
- @NotNull
+ @Nonnull
@Override
- public Result apply(@NotNull SnapshotAtomicOperationInputCommand command, @NotNull Saga saga) {
+ public Result apply(@Nonnull SnapshotAtomicOperationInputCommand command, @Nonnull Saga saga) {
// We happily don't need to do anything here. This action just snapshots our input data.
return new Result(command.nextCommand, Collections.emptyList());
}
@@ -64,7 +63,7 @@ public class SnapshotAtomicOperationInput
@NonFinal private EventMetadata metadata;
@Override
- public void setMetadata(@NotNull EventMetadata eventMetadata) {
+ public void setMetadata(@Nonnull EventMetadata eventMetadata) {
this.metadata = eventMetadata;
}

diff:
@@ -50,8 +50,8 @@ import java.io.UncheckedIOException;
import java.util.List;
import java.util.Optional;
import java.util.Random;
+import javax.annotation.Nonnull;
import lombok.extern.slf4j.Slf4j;
-import org.jetbrains.annotations.NotNull;
@Slf4j
public class StatefullyUpdateBootImageAtomicOperation extends GoogleAtomicOperation<Void> {
@@ -160,7 +160,7 @@ public class StatefullyUpdateBootImageAtomicOperation extends GoogleAtomicOperat
}
}
- @NotNull
+ @Nonnull
private Image getImage(Task task, GoogleNamedAccountCredentials credentials) throws IOException {
task.updateStatus(BASE_PHASE, "Looking up image " + description.getBootImage());

diff:
@@ -33,7 +33,6 @@ import javax.annotation.Nullable;
import lombok.Builder;
import lombok.Value;
import lombok.experimental.NonFinal;
-import org.jetbrains.annotations.NotNull;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
@@ -52,9 +51,9 @@ public class AttachTitusServiceLoadBalancers extends AbstractTitusDeployAction
this.titusClientProvider = titusClientProvider1;
}
- @NotNull
+ @Nonnull
@Override
- public Result apply(@NotNull AttachTitusServiceLoadBalancersCommand command, @NotNull Saga saga) {
+ public Result apply(@Nonnull AttachTitusServiceLoadBalancersCommand command, @Nonnull Saga saga) {
final TitusDeployDescription description = command.description;
prepareDeployDescription(description);

diff:
@@ -45,7 +45,6 @@ import javax.annotation.Nonnull;
import lombok.Builder;
import lombok.Value;
import lombok.experimental.NonFinal;
-import org.jetbrains.annotations.NotNull;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
@@ -63,9 +62,9 @@ public class CopyTitusServiceScalingPolicies extends AbstractTitusDeployAction
super(accountCredentialsRepository, titusClientProvider);
}
- @NotNull
+ @Nonnull
@Override
- public Result apply(@NotNull CopyTitusServiceScalingPoliciesCommand command, @NotNull Saga saga) {
+ public Result apply(@Nonnull CopyTitusServiceScalingPoliciesCommand command, @Nonnull Saga saga) {
final TitusDeployDescription description = command.description;
prepareDeployDescription(description);

diff:
@@ -63,7 +63,6 @@ import javax.annotation.Nullable;
import lombok.Builder;
import lombok.Value;
import lombok.experimental.NonFinal;
-import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
@@ -117,9 +116,9 @@ public class PrepareTitusDeploy extends AbstractTitusDeployAction
return input;
}
- @NotNull
+ @Nonnull
@Override
- public Result apply(@NotNull PrepareTitusDeployCommand command, @NotNull Saga saga) {
+ public Result apply(@Nonnull PrepareTitusDeployCommand command, @Nonnull Saga saga) {
final TitusDeployDescription description = command.description;
prepareDeployDescription(description);

diff:
@@ -45,7 +45,6 @@ import javax.annotation.Nonnull;
import lombok.Builder;
import lombok.Value;
import lombok.experimental.NonFinal;
-import org.jetbrains.annotations.NotNull;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
@@ -73,9 +72,9 @@ public class SubmitTitusJob extends AbstractTitusDeployAction
* references inside of the lambda. This should really be refactored so that pattern isn't
* necessary. It's really gross as-is.
*/
- @NotNull
+ @Nonnull
@Override
- public Result apply(@NotNull SubmitTitusJobCommand command, @NotNull Saga saga) {
+ public Result apply(@Nonnull SubmitTitusJobCommand command, @Nonnull Saga saga) {
final TitusDeployDescription description = command.description;
prepareDeployDescription(description);

diff:
@@ -22,8 +22,8 @@ import com.netflix.spinnaker.clouddriver.titus.TitusUtils;
import com.netflix.spinnaker.clouddriver.titus.deploy.actions.SubmitTitusJob;
import com.netflix.spinnaker.clouddriver.titus.deploy.description.TitusDeployDescription;
import com.netflix.spinnaker.clouddriver.titus.deploy.events.TitusJobSubmitted;
-import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Component;
@@ -39,7 +39,7 @@ public class TitusDeployCompletionHandler implements SagaCompletionHandler<Titus
@Nullable
@Override
- public TitusDeploymentResult handle(@NotNull Saga completedSaga) {
+ public TitusDeploymentResult handle(@Nonnull Saga completedSaga) {
final TitusDeployDescription description =
completedSaga.getEvent(SubmitTitusJob.SubmitTitusJobCommand.class).getDescription();

diff:
@@ -19,15 +19,15 @@ package com.netflix.spinnaker.clouddriver.titus.deploy.handlers;
import com.netflix.spinnaker.clouddriver.saga.flow.SagaExceptionHandler;
import com.netflix.spinnaker.clouddriver.titus.TitusException;
import io.grpc.StatusRuntimeException;
-import org.jetbrains.annotations.NotNull;
+import javax.annotation.Nonnull;
import org.springframework.stereotype.Component;
@Component
public class TitusExceptionHandler implements SagaExceptionHandler {
- @NotNull
+ @Nonnull
@Override
- public Exception handle(@NotNull Exception exception) {
+ public Exception handle(@Nonnull Exception exception) {
if (exception instanceof StatusRuntimeException) {
StatusRuntimeException statusRuntimeException = (StatusRuntimeException) exception;
return new TitusException(statusRuntimeException, isRetryable(statusRuntimeException));

original_message: chore(core): standardize on javax nonnull and nullable annotations (#4242)


message:
fix(cf): remove cf secondary cache expiry to prevent expensive api calls
spinnaker/spinnaker#5286

diff:
@@ -42,7 +42,6 @@ import java.io.InputStream;
import java.nio.charset.Charset;
import java.util.*;
import java.util.concurrent.ExecutionException;
-import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
@@ -66,7 +65,6 @@ public class Applications {
private final LoadingCache<String, CloudFoundryServerGroup> serverGroupCache =
CacheBuilder.newBuilder()
- .expireAfterWrite(5, TimeUnit.MINUTES)
.build(
new CacheLoader<String, CloudFoundryServerGroup>() {
@Override

original_message:
fix(cf): remove cf secondary cache expiry to prevent expensive api calls (#4244)

spinnaker/spinnaker#5286


diff:
@@ -31,6 +31,7 @@ import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.Kube
import com.netflix.spinnaker.clouddriver.model.SecurityGroup;
import com.netflix.spinnaker.clouddriver.model.SecurityGroupSummary;
import com.netflix.spinnaker.clouddriver.model.securitygroups.Rule;
+import io.kubernetes.client.custom.IntOrString;
import io.kubernetes.client.models.V1NetworkPolicy;
import io.kubernetes.client.models.V1NetworkPolicyEgressRule;
import io.kubernetes.client.models.V1NetworkPolicyIngressRule;
@@ -43,6 +44,7 @@ import java.util.Set;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.stream.Collectors;
+import javax.annotation.Nullable;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
@@ -143,10 +145,13 @@ public class KubernetesV2SecurityGroup extends ManifestBasedModel implements Sec
}
private static Rule fromPolicyPort(V1NetworkPolicyPort policyPort) {
- String port = policyPort.getPort().toString();
+ IntOrString port = policyPort.getPort();
return new PortRule()
.setProtocol(policyPort.getProtocol())
- .setPortRanges(new TreeSet<>(Collections.singletonList(new StringPortRange(port))));
+ .setPortRanges(
+ port == null
+ ? null
+ : new TreeSet<>(Collections.singletonList(new StringPortRange(port.toString()))));
}
@Data
@@ -160,8 +165,8 @@ public class KubernetesV2SecurityGroup extends ManifestBasedModel implements Sec
@Data
private static class PortRule implements Rule {
- private SortedSet<PortRange> portRanges;
- private String protocol;
+ @Nullable private SortedSet<PortRange> portRanges;
+ @Nullable private String protocol;
}
@EqualsAndHashCode(callSuper = true)

original_message: fix(kubernetes): permit null properties of NetworkPolicy PortRules (#4245)


diff:
@@ -119,7 +119,8 @@ public class KubernetesV2InstanceProvider
@Nonnull
private List<ContainerLog> getPodLogs(
@Nonnull KubernetesV2Credentials credentials, @Nonnull V1Pod pod) {
- List<V1Container> initContainers = pod.getSpec().getInitContainers();
+ List<V1Container> initContainers =
+ Optional.ofNullable(pod.getSpec().getInitContainers()).orElse(Collections.emptyList());
List<V1Container> containers = pod.getSpec().getContainers();
return Stream.concat(initContainers.stream(), containers.stream())

original_message: fix(kubernetes): do not throw NPE when no initContainers (#4251)


diff:
@@ -20,6 +20,7 @@ package com.netflix.spinnaker.clouddriver.kubernetes.v2.caching;
import static com.netflix.spinnaker.clouddriver.kubernetes.v2.caching.Keys.Kind.KUBERNETES_METRIC;
import com.fasterxml.jackson.annotation.JsonCreator;
+import com.netflix.spinnaker.clouddriver.kubernetes.KubernetesCloudProvider;
import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesKind;
import com.netflix.spinnaker.clouddriver.kubernetes.v2.description.manifest.KubernetesManifest;
import java.util.Arrays;
@@ -165,7 +166,7 @@ public class Keys {
@EqualsAndHashCode
public abstract static class CacheKey {
- private static final String provider = "kubernetes.v2";
+ @Getter private final String provider = KubernetesCloudProvider.ID;
public abstract String getGroup();

original_message: fix(kubernetes): return provider field with kubernetes /search results (#4249)


message: chore(kubernetes): bump kubernetes java client for v1

diff:
@@ -19,7 +19,7 @@ dependencies {
implementation "com.netflix.spinnaker.kork:kork-config"
implementation "com.netflix.spinnaker.moniker:moniker"
implementation "io.fabric8:kubernetes-client:4.6.0"
- implementation "io.kubernetes:client-java:5.0.0"
+ implementation "io.kubernetes:client-java:7.0.0"
implementation "org.springframework.boot:spring-boot-actuator"
implementation "org.springframework.boot:spring-boot-starter-web"

diff:
@@ -25,23 +25,23 @@ import com.netflix.spinnaker.clouddriver.kubernetes.v1.security.KubernetesApiCli
import com.netflix.spinnaker.clouddriver.kubernetes.v1.deploy.KubernetesUtil
import com.netflix.spinnaker.clouddriver.kubernetes.v1.deploy.exception.KubernetesClientOperationException
import groovy.util.logging.Slf4j
-import io.kubernetes.client.ApiClient
-import io.kubernetes.client.ApiException
-import io.kubernetes.client.Configuration
-import io.kubernetes.client.apis.AppsV1beta1Api
-import io.kubernetes.client.apis.AutoscalingV1Api
-import io.kubernetes.client.apis.CoreV1Api
-import io.kubernetes.client.apis.ExtensionsV1beta1Api
-import io.kubernetes.client.models.V1DeleteOptions
-import io.kubernetes.client.models.V1HorizontalPodAutoscaler
-import io.kubernetes.client.models.V1NamespaceList
-import io.kubernetes.client.models.V1Pod
-import io.kubernetes.client.models.V1PodList
-import io.kubernetes.client.models.V1Status
-import io.kubernetes.client.models.V1beta1DaemonSet
-import io.kubernetes.client.models.V1beta1DaemonSetList
-import io.kubernetes.client.models.V1beta1StatefulSet
-import io.kubernetes.client.models.V1beta1StatefulSetList
+import io.kubernetes.client.openapi.ApiClient
+import io.kubernetes.client.openapi.ApiException
+import io.kubernetes.client.openapi.Configuration
+import io.kubernetes.client.openapi.apis.AppsV1beta1Api
+import io.kubernetes.client.openapi.apis.AutoscalingV1Api
+import io.kubernetes.client.openapi.apis.CoreV1Api
+import io.kubernetes.client.openapi.apis.ExtensionsV1beta1Api
+import io.kubernetes.client.openapi.models.V1DeleteOptions
+import io.kubernetes.client.openapi.models.V1HorizontalPodAutoscaler
+import io.kubernetes.client.openapi.models.V1NamespaceList
+import io.kubernetes.client.openapi.models.V1Pod
+import io.kubernetes.client.openapi.models.V1PodList
+import io.kubernetes.client.openapi.models.V1Status
+import io.kubernetes.client.openapi.models.V1beta1DaemonSet
+import io.kubernetes.client.openapi.models.V1beta1DaemonSetList
+import io.kubernetes.client.openapi.models.V1beta1StatefulSet
+import io.kubernetes.client.openapi.models.V1beta1StatefulSetList
import java.util.concurrent.TimeUnit

diff:
@@ -50,49 +50,49 @@ import com.netflix.spinnaker.clouddriver.kubernetes.v1.model.KubernetesControlle
import groovy.transform.CompileStatic
import groovy.util.logging.Slf4j
import io.kubernetes.client.custom.IntOrString
-import io.kubernetes.client.models.V1Capabilities
-import io.kubernetes.client.models.V1ConfigMapVolumeSource
-import io.kubernetes.client.models.V1Container
-import io.kubernetes.client.models.V1ContainerPort
-import io.kubernetes.client.models.V1CrossVersionObjectReference
-import io.kubernetes.client.models.V1EmptyDirVolumeSource
-import io.kubernetes.client.models.V1EnvVar
-import io.kubernetes.client.models.V1EnvVarSource
-import io.kubernetes.client.models.V1ExecAction
-import io.kubernetes.client.models.V1HTTPGetAction
-import io.kubernetes.client.models.V1HTTPHeader
-import io.kubernetes.client.models.V1Handler
-import io.kubernetes.client.models.V1HorizontalPodAutoscaler
-import io.kubernetes.client.models.V1HorizontalPodAutoscalerSpec
-import io.kubernetes.client.models.V1HostPathVolumeSource
-import io.kubernetes.client.models.V1KeyToPath
-import io.kubernetes.client.models.V1LabelSelector
-import io.kubernetes.client.models.V1Lifecycle
-import io.kubernetes.client.models.V1ObjectFieldSelector
-import io.kubernetes.client.models.V1ObjectMeta
-import io.kubernetes.client.models.V1ObjectReference
-import io.kubernetes.client.models.V1PersistentVolumeClaim
-import io.kubernetes.client.models.V1PersistentVolumeClaimSpec
-import io.kubernetes.client.models.V1PersistentVolumeClaimVolumeSource
-import io.kubernetes.client.models.V1PodSpec
-import io.kubernetes.client.models.V1PodTemplateSpec
-import io.kubernetes.client.models.V1Probe
-import io.kubernetes.client.models.V1ResourceFieldSelector
-import io.kubernetes.client.models.V1ResourceRequirements
-import io.kubernetes.client.models.V1SELinuxOptions
-import io.kubernetes.client.models.V1SecretVolumeSource
-import io.kubernetes.client.models.V1SecurityContext
-import io.kubernetes.client.models.V1TCPSocketAction
-import io.kubernetes.client.models.V1Volume
-import io.kubernetes.client.models.V1VolumeMount
-import io.kubernetes.client.models.V1beta1DaemonSet
-import io.kubernetes.client.models.V1beta1DaemonSetSpec
-import io.kubernetes.client.models.V1beta1DaemonSetUpdateStrategy
-import io.kubernetes.client.models.V1beta1RollingUpdateDaemonSet
-import io.kubernetes.client.models.V1beta1RollingUpdateStatefulSetStrategy
-import io.kubernetes.client.models.V1beta1StatefulSet
-import io.kubernetes.client.models.V1beta1StatefulSetSpec
-import io.kubernetes.client.models.V1beta1StatefulSetUpdateStrategy
+import io.kubernetes.client.openapi.models.V1Capabilities
+import io.kubernetes.client.openapi.models.V1ConfigMapVolumeSource
+import io.kubernetes.client.openapi.models.V1Container
+import io.kubernetes.client.openapi.models.V1ContainerPort
+import io.kubernetes.client.openapi.models.V1CrossVersionObjectReference
+import io.kubernetes.client.openapi.models.V1EmptyDirVolumeSource
+import io.kubernetes.client.openapi.models.V1EnvVar
+import io.kubernetes.client.openapi.models.V1EnvVarSource
+import io.kubernetes.client.openapi.models.V1ExecAction
+import io.kubernetes.client.openapi.models.V1HTTPGetAction
+import io.kubernetes.client.openapi.models.V1HTTPHeader
+import io.kubernetes.client.openapi.models.V1Handler
+import io.kubernetes.client.openapi.models.V1HorizontalPodAutoscaler
+import io.kubernetes.client.openapi.models.V1HorizontalPodAutoscalerSpec
+import io.kubernetes.client.openapi.models.V1HostPathVolumeSource
+import io.kubernetes.client.openapi.models.V1KeyToPath
+import io.kubernetes.client.openapi.models.V1LabelSelector
+import io.kubernetes.client.openapi.models.V1Lifecycle
+import io.kubernetes.client.openapi.models.V1ObjectFieldSelector
+import io.kubernetes.client.openapi.models.V1ObjectMeta
+import io.kubernetes.client.openapi.models.V1ObjectReference
+import io.kubernetes.client.openapi.models.V1PersistentVolumeClaim
+import io.kubernetes.client.openapi.models.V1PersistentVolumeClaimSpec
+import io.kubernetes.client.openapi.models.V1PersistentVolumeClaimVolumeSource
+import io.kubernetes.client.openapi.models.V1PodSpec
+import io.kubernetes.client.openapi.models.V1PodTemplateSpec
+import io.kubernetes.client.openapi.models.V1Probe
+import io.kubernetes.client.openapi.models.V1ResourceFieldSelector
+import io.kubernetes.client.openapi.models.V1ResourceRequirements
+import io.kubernetes.client.openapi.models.V1SELinuxOptions
+import io.kubernetes.client.openapi.models.V1SecretVolumeSource
+import io.kubernetes.client.openapi.models.V1SecurityContext
+import io.kubernetes.client.openapi.models.V1TCPSocketAction
+import io.kubernetes.client.openapi.models.V1Volume
+import io.kubernetes.client.openapi.models.V1VolumeMount
+import io.kubernetes.client.openapi.models.V1beta1DaemonSet
+import io.kubernetes.client.openapi.models.V1beta1DaemonSetSpec
+import io.kubernetes.client.openapi.models.V1beta1DaemonSetUpdateStrategy
+import io.kubernetes.client.openapi.models.V1beta1RollingUpdateDaemonSet
+import io.kubernetes.client.openapi.models.V1beta1RollingUpdateStatefulSetStrategy
+import io.kubernetes.client.openapi.models.V1beta1StatefulSet
+import io.kubernetes.client.openapi.models.V1beta1StatefulSetSpec
+import io.kubernetes.client.openapi.models.V1beta1StatefulSetUpdateStrategy
import io.kubernetes.client.util.Yaml
import org.slf4j.Logger
import org.slf4j.LoggerFactory

diff:
package com.netflix.spinnaker.clouddriver.kubernetes.v1.deploy.exception
import groovy.transform.InheritConstructors
-import io.kubernetes.client.ApiException
+import io.kubernetes.client.openapi.ApiException
@InheritConstructors
class KubernetesClientOperationException extends RuntimeException {

diff:
@@ -36,7 +36,7 @@ import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder
import io.fabric8.kubernetes.api.model.apps.DeploymentFluentImpl
import io.fabric8.kubernetes.api.model.apps.DoneableDeployment
import io.fabric8.kubernetes.api.model.apps.ReplicaSetBuilder
-import io.kubernetes.client.models.V1Pod
+import io.kubernetes.client.openapi.models.V1Pod
class DeployKubernetesAtomicOperation implements AtomicOperation<DeploymentResult> {
private static final String BASE_PHASE = "DEPLOY"

diff:
@@ -18,7 +18,7 @@ package com.netflix.spinnaker.clouddriver.kubernetes.v1.model
import io.fabric8.kubernetes.api.model.HasMetadata
import io.fabric8.kubernetes.api.model.ObjectMeta
-import io.kubernetes.client.models.V1ObjectMeta
+import io.kubernetes.client.openapi.models.V1ObjectMeta
class KubernetesControllerConverter implements HasMetadata {
String kind

diff:
@@ -34,8 +34,8 @@ import io.fabric8.kubernetes.api.model.HorizontalPodAutoscaler
import io.fabric8.kubernetes.api.model.ReplicationController
import io.fabric8.kubernetes.api.model.apps.ReplicaSet
import io.fabric8.kubernetes.client.internal.SerializationUtils
-import io.kubernetes.client.models.V1beta1DaemonSet
-import io.kubernetes.client.models.V1beta1StatefulSet
+import io.kubernetes.client.openapi.models.V1beta1DaemonSet
+import io.kubernetes.client.openapi.models.V1beta1StatefulSet
import static com.netflix.spinnaker.clouddriver.kubernetes.v1.deploy.KubernetesUtil.ENABLE_DISABLE_ANNOTATION

diff:
@@ -37,9 +37,9 @@ import com.netflix.spinnaker.clouddriver.kubernetes.v1.provider.view.MutableCach
import com.netflix.spinnaker.clouddriver.kubernetes.v1.security.KubernetesV1Credentials
import groovy.util.logging.Slf4j
import io.fabric8.kubernetes.api.model.Event
-import io.kubernetes.client.models.V1PodList
-import io.kubernetes.client.models.V1beta1DaemonSet
-import io.kubernetes.client.models.V1beta1StatefulSet
+import io.kubernetes.client.openapi.models.V1PodList
+import io.kubernetes.client.openapi.models.V1beta1DaemonSet
+import io.kubernetes.client.openapi.models.V1beta1StatefulSet
import static com.netflix.spinnaker.cats.agent.AgentDataType.Authority.AUTHORITATIVE
import static com.netflix.spinnaker.cats.agent.AgentDataType.Authority.INFORMATIVE

diff:
package com.netflix.spinnaker.clouddriver.kubernetes.v1.security
import groovy.util.logging.Slf4j
-import io.kubernetes.client.ApiClient
+import io.kubernetes.client.openapi.ApiClient
import io.kubernetes.client.util.Config
import io.kubernetes.client.util.KubeConfig
import org.apache.commons.lang3.StringUtils


message:
fix(cf): reduce results per page requests to lower timeout frequency
spinnaker/spinnaker#5286

diff:
@@ -95,7 +95,7 @@ public class Applications {
public List<CloudFoundryApplication> all() {
List<Application> newCloudFoundryAppList =
- collectPages("applications", page -> api.all(page, 5000, null, null));
+ collectPages("applications", page -> api.all(page, 500, null, null));
// Evict Records from `cache` that are no longer in the foundation
List<String> availableAppIds =

diff:
@@ -76,6 +76,6 @@ public class CloudFoundryApiException extends RuntimeException {
+ ". raw response body: "
+ response.getBody();
})
- .orElse("error, url :" + retrofitError.getUrl() + " body: " + retrofitError.getBody());
+ .orElse("error, url :" + retrofitError.getUrl() + " cause: " + retrofitError.getCause());
}
}

diff:
@@ -59,7 +59,7 @@ public class Routes {
private LoadingCache<String, List<RouteMapping>> routeMappings =
CacheBuilder.newBuilder()
- .expireAfterWrite(30, TimeUnit.SECONDS)
+ .expireAfterWrite(3, TimeUnit.MINUTES)
.build(
new CacheLoader<String, List<RouteMapping>>() {
@Override
@@ -143,7 +143,7 @@ public class Routes {
public List<CloudFoundryLoadBalancer> all() throws CloudFoundryApiException {
List<Resource<Route>> routeResources =
- collectPageResources("routes", pg -> api.all(pg, 5000, null));
+ collectPageResources("routes", pg -> api.all(pg, 500, null));
List<CloudFoundryLoadBalancer> loadBalancers = new ArrayList<>(routeResources.size());
for (Resource<Route> routeResource : routeResources) {
loadBalancers.add(map(routeResource));

original_message:
fix(cf): reduce results per page requests to lower timeout frequency (#4271)

spinnaker/spinnaker#5286


message:
feat(ecs): Refactor TaskHealthCachingAgent to use TargetHealthCache
* feat(ecs): Refactor TaskHealthCachingAgent to use TargetHealthCache
* Determines Task health with cached ALB target healths vs.
calls to Elastic Load Balancing
* Return status_unknown for targets not found

diff:
@@ -31,10 +31,6 @@ import com.amazonaws.services.ecs.model.NetworkBinding;
import com.amazonaws.services.ecs.model.NetworkInterface;
import com.amazonaws.services.ecs.model.PortMapping;
import com.amazonaws.services.ecs.model.TaskDefinition;
-import com.amazonaws.services.elasticloadbalancingv2.AmazonElasticLoadBalancing;
-import com.amazonaws.services.elasticloadbalancingv2.model.DescribeTargetHealthRequest;
-import com.amazonaws.services.elasticloadbalancingv2.model.DescribeTargetHealthResult;
-import com.amazonaws.services.elasticloadbalancingv2.model.TargetDescription;
import com.amazonaws.services.elasticloadbalancingv2.model.TargetHealthDescription;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.netflix.spinnaker.cats.agent.AgentDataType;
@@ -45,14 +41,8 @@ import com.netflix.spinnaker.clouddriver.aws.security.AmazonClientProvider;
import com.netflix.spinnaker.clouddriver.aws.security.NetflixAmazonCredentials;
import com.netflix.spinnaker.clouddriver.core.provider.agent.HealthProvidingCachingAgent;
import com.netflix.spinnaker.clouddriver.ecs.cache.Keys;
-import com.netflix.spinnaker.clouddriver.ecs.cache.client.ContainerInstanceCacheClient;
-import com.netflix.spinnaker.clouddriver.ecs.cache.client.ServiceCacheClient;
-import com.netflix.spinnaker.clouddriver.ecs.cache.client.TaskCacheClient;
-import com.netflix.spinnaker.clouddriver.ecs.cache.client.TaskDefinitionCacheClient;
-import com.netflix.spinnaker.clouddriver.ecs.cache.model.ContainerInstance;
-import com.netflix.spinnaker.clouddriver.ecs.cache.model.Service;
-import com.netflix.spinnaker.clouddriver.ecs.cache.model.Task;
-import com.netflix.spinnaker.clouddriver.ecs.cache.model.TaskHealth;
+import com.netflix.spinnaker.clouddriver.ecs.cache.client.*;
+import com.netflix.spinnaker.clouddriver.ecs.cache.model.*;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -107,8 +97,8 @@ public class TaskHealthCachingAgent extends AbstractEcsCachingAgent<TaskHealth>
new TaskDefinitionCacheClient(providerCache, objectMapper);
ServiceCacheClient serviceCacheClient = new ServiceCacheClient(providerCache, objectMapper);
- AmazonElasticLoadBalancing amazonloadBalancing =
- amazonClientProvider.getAmazonElasticLoadBalancingV2(account, region, false);
+ TargetHealthCacheClient targetHealthCacheClient =
+ new TargetHealthCacheClient(providerCache, objectMapper);
ContainerInstanceCacheClient containerInstanceCacheClient =
new ContainerInstanceCacheClient(providerCache);
@@ -148,7 +138,7 @@ public class TaskHealthCachingAgent extends AbstractEcsCachingAgent<TaskHealth>
if (task.getContainers().get(0).getNetworkBindings().size() >= 1) {
taskHealth =
inferHealthNetworkBindedContainer(
- amazonloadBalancing,
+ targetHealthCacheClient,
task,
containerInstance,
serviceName,
@@ -157,7 +147,7 @@ public class TaskHealthCachingAgent extends AbstractEcsCachingAgent<TaskHealth>
} else {
taskHealth =
inferHealthNetworkInterfacedContainer(
- amazonloadBalancing, task, serviceName, service, taskDefinition);
+ targetHealthCacheClient, task, serviceName, service, taskDefinition);
}
log.debug("Task Health contains the following elements: {}", taskHealth);
@@ -172,7 +162,7 @@ public class TaskHealthCachingAgent extends AbstractEcsCachingAgent<TaskHealth>
}
private TaskHealth inferHealthNetworkInterfacedContainer(
- AmazonElasticLoadBalancing amazonloadBalancing,
+ TargetHealthCacheClient targetHealthCacheClient,
Task task,
String serviceName,
Service loadBalancerService,
@@ -205,7 +195,7 @@ public class TaskHealthCachingAgent extends AbstractEcsCachingAgent<TaskHealth>
overallTaskHealth =
describeTargetHealth(
- amazonloadBalancing,
+ targetHealthCacheClient,
task,
loadBalancerService,
serviceName,
@@ -231,12 +221,14 @@ public class TaskHealthCachingAgent extends AbstractEcsCachingAgent<TaskHealth>
private TaskHealth makeTaskHealth(
Task task, String serviceName, TargetHealthDescription healthDescription) {
+ String targetHealth = STATUS_UNKNOWN;
+ if (healthDescription != null) {
log.debug("Task target health is: {}", healthDescription.getTargetHealth());
- String targetHealth =
+ targetHealth =
healthDescription.getTargetHealth().getState().equals("healthy")
? STATUS_UP
: STATUS_UNKNOWN;
-
+ }
TaskHealth taskHealth = new TaskHealth();
taskHealth.setType("loadBalancer");
taskHealth.setState(targetHealth);
@@ -249,7 +241,7 @@ public class TaskHealthCachingAgent extends AbstractEcsCachingAgent<TaskHealth>
}
private TaskHealth inferHealthNetworkBindedContainer(
- AmazonElasticLoadBalancing amazonloadBalancing,
+ TargetHealthCacheClient targetHealthCacheClient,
Task task,
ContainerInstance containerInstance,
String serviceName,
@@ -286,7 +278,7 @@ public class TaskHealthCachingAgent extends AbstractEcsCachingAgent<TaskHealth>
overallTaskHealth =
describeTargetHealth(
- amazonloadBalancing,
+ targetHealthCacheClient,
task,
loadBalancerService,
serviceName,
@@ -299,8 +291,20 @@ public class TaskHealthCachingAgent extends AbstractEcsCachingAgent<TaskHealth>
return overallTaskHealth;
}
+ private TargetHealthDescription findHealthDescription(
+ List<TargetHealthDescription> targetHealths, String targetId, Integer targetPort) {
+
+ return targetHealths.stream()
+ .filter(
+ h ->
+ h.getTarget().getId().equals(targetId)
+ && h.getTarget().getPort().equals(targetPort))
+ .findFirst()
+ .orElse(null);
+ }
+
private TaskHealth describeTargetHealth(
- AmazonElasticLoadBalancing amazonloadBalancing,
+ TargetHealthCacheClient targetHealthCacheClient,
Task task,
Service loadBalancerService,
String serviceName,
@@ -308,25 +312,31 @@ public class TaskHealthCachingAgent extends AbstractEcsCachingAgent<TaskHealth>
String targetId,
Integer targetPort,
TaskHealth overallTaskHealth) {
- DescribeTargetHealthResult describeTargetHealthResult =
- amazonloadBalancing.describeTargetHealth(
- new DescribeTargetHealthRequest()
- .withTargetGroupArn(targetGroupArn)
- .withTargets(new TargetDescription().withId(targetId).withPort(targetPort)));
-
- if (describeTargetHealthResult.getTargetHealthDescriptions().isEmpty()) {
- log.debug("Target health description is empty");
+
+ String targetHealthKey = Keys.getTargetHealthKey(accountName, region, targetGroupArn);
+ EcsTargetHealth targetHealth = targetHealthCacheClient.get(targetHealthKey);
+
+ if (targetHealth == null) {
+ log.debug("Cached EcsTargetHealth is empty for targetGroup {}", targetGroupArn);
evictStaleData(task, loadBalancerService);
- return overallTaskHealth;
+ return makeTaskHealth(task, serviceName, null);
}
+ TargetHealthDescription targetHealthDescription =
+ findHealthDescription(targetHealth.getTargetHealthDescriptions(), targetId, targetPort);
+ if (targetHealthDescription == null) {
log.debug(
- "Target health description is not empty and has a size of {}",
- describeTargetHealthResult.getTargetHealthDescriptions().size());
- TargetHealthDescription healthDescription =
- describeTargetHealthResult.getTargetHealthDescriptions().get(0);
+ "TargetHealthDescription is empty on targetGroup '{}' for {}:{}",
+ targetGroupArn,
+ targetId,
+ targetPort);
+ evictStaleData(task, loadBalancerService);
+ return makeTaskHealth(task, serviceName, null);
+ }
+
+ log.debug("Retrieved health of targetId {} for targetGroup {}", targetId, targetGroupArn);
- TaskHealth taskHealth = makeTaskHealth(task, serviceName, healthDescription);
+ TaskHealth taskHealth = makeTaskHealth(task, serviceName, targetHealthDescription);
if ((overallTaskHealth == null) || (taskHealth.getState().equals(STATUS_UNKNOWN))) {
return taskHealth;
}

diff:
@@ -25,6 +25,7 @@ import com.amazonaws.services.ecs.model.NetworkBinding
import com.amazonaws.services.ecs.model.PortMapping
import com.amazonaws.services.elasticloadbalancingv2.AmazonElasticLoadBalancing
import com.amazonaws.services.elasticloadbalancingv2.model.DescribeTargetHealthResult
+import com.amazonaws.services.elasticloadbalancingv2.model.TargetDescription
import com.amazonaws.services.elasticloadbalancingv2.model.TargetHealth
import com.amazonaws.services.elasticloadbalancingv2.model.TargetHealthDescription
import com.amazonaws.services.elasticloadbalancingv2.model.TargetHealthStateEnum
@@ -38,6 +39,7 @@ import spock.lang.Specification
import spock.lang.Subject
import static com.netflix.spinnaker.clouddriver.core.provider.agent.Namespace.HEALTH
+import static com.netflix.spinnaker.clouddriver.ecs.cache.Keys.Namespace.TARGET_HEALTHS
import static com.netflix.spinnaker.clouddriver.ecs.cache.Keys.Namespace.TASKS
import static com.netflix.spinnaker.clouddriver.ecs.cache.Keys.Namespace.TASK_DEFINITIONS
@@ -64,10 +66,13 @@ class TaskHealthCacheSpec extends Specification {
def healthKey = Keys.getTaskHealthKey(CommonCachingAgent.ACCOUNT, CommonCachingAgent.REGION, CommonCachingAgent.TASK_ID_1)
def serviceKey = Keys.getServiceKey(CommonCachingAgent.ACCOUNT, CommonCachingAgent.REGION, CommonCachingAgent.SERVICE_NAME_1)
def containerInstanceKey = Keys.getContainerInstanceKey(CommonCachingAgent.ACCOUNT, CommonCachingAgent.REGION, CommonCachingAgent.CONTAINER_INSTANCE_ARN_1)
+ def targetHealthKey = Keys.getTargetHealthKey(CommonCachingAgent.ACCOUNT, CommonCachingAgent.REGION, targetGroupArn)
ObjectMapper mapper = new ObjectMapper()
Map<String, Object> containerMap = mapper.convertValue(new Container().withNetworkBindings(new NetworkBinding().withContainerPort(1338).withHostPort(1338)), Map.class)
Map<String, Object> loadbalancerMap = mapper.convertValue(new LoadBalancer().withTargetGroupArn(targetGroupArn).withContainerPort(1338), Map.class)
+ Map<String, Object> targetHealthMap = mapper.convertValue(
+ new TargetHealthDescription().withTarget(new TargetDescription().withId(CommonCachingAgent.EC2_INSTANCE_ID_1).withPort(1338)).withTargetHealth(new TargetHealth().withState(TargetHealthStateEnum.Healthy)), Map.class)
def taskAttributes = [
taskId : CommonCachingAgent.TASK_ID_1,
@@ -98,6 +103,14 @@ class TaskHealthCacheSpec extends Specification {
def containerInstanceCache = new DefaultCacheData(containerInstanceKey, containerInstanceAttributes, Collections.emptyMap())
providerCache.get(Keys.Namespace.CONTAINER_INSTANCES.toString(), containerInstanceKey) >> containerInstanceCache
+ def targetHealthAttributes = [
+ targetGroupArn : targetGroupArn,
+ targetHealthDescriptions : Collections.singletonList(targetHealthMap)
+ ]
+
+ def targetHealthCache = new DefaultCacheData(targetHealthKey, targetHealthAttributes, Collections.emptyMap())
+ providerCache.get(TARGET_HEALTHS.toString(), targetHealthKey) >> targetHealthCache
+
DescribeTargetHealthResult describeTargetHealthResult = new DescribeTargetHealthResult().withTargetHealthDescriptions(
new TargetHealthDescription().withTargetHealth(new TargetHealth().withState(TargetHealthStateEnum.Healthy))
)

diff:
@@ -28,6 +28,7 @@ import com.amazonaws.services.ecs.model.TaskDefinition
import com.amazonaws.services.elasticloadbalancingv2.AmazonElasticLoadBalancing
import com.amazonaws.services.elasticloadbalancingv2.model.DescribeTargetHealthRequest
import com.amazonaws.services.elasticloadbalancingv2.model.DescribeTargetHealthResult
+import com.amazonaws.services.elasticloadbalancingv2.model.TargetDescription
import com.amazonaws.services.elasticloadbalancingv2.model.TargetHealth
import com.amazonaws.services.elasticloadbalancingv2.model.TargetHealthDescription
import com.amazonaws.services.elasticloadbalancingv2.model.TargetHealthStateEnum
@@ -43,6 +44,7 @@ import spock.lang.Specification
import spock.lang.Subject
import static com.netflix.spinnaker.clouddriver.core.provider.agent.Namespace.HEALTH
+import static com.netflix.spinnaker.clouddriver.ecs.cache.Keys.Namespace.TARGET_HEALTHS
import static com.netflix.spinnaker.clouddriver.ecs.cache.Keys.Namespace.TASKS
import static com.netflix.spinnaker.clouddriver.ecs.cache.Keys.Namespace.TASK_DEFINITIONS
@@ -51,7 +53,6 @@ class TaskHealthCachingAgentSpec extends Specification {
def clientProvider = Mock(AmazonClientProvider)
def providerCache = Mock(ProviderCache)
def credentialsProvider = Mock(AWSCredentialsProvider)
- def amazonloadBalancing = Mock(AmazonElasticLoadBalancing)
def targetGroupArn = 'arn:aws:elasticloadbalancing:' + CommonCachingAgent.REGION + ':769716316905:targetgroup/test-target-group/9e8997b7cff00c62'
ObjectMapper mapper = new ObjectMapper()
@@ -60,13 +61,21 @@ class TaskHealthCachingAgentSpec extends Specification {
TaskHealthCachingAgent agent = new TaskHealthCachingAgent(CommonCachingAgent.netflixAmazonCredentials, CommonCachingAgent.REGION, clientProvider, credentialsProvider, mapper)
def setup() {
- clientProvider.getAmazonElasticLoadBalancingV2(_, _, _) >> amazonloadBalancing
def serviceKey = Keys.getServiceKey(CommonCachingAgent.ACCOUNT, CommonCachingAgent.REGION, CommonCachingAgent.SERVICE_NAME_1)
def containerInstanceKey = Keys.getContainerInstanceKey(CommonCachingAgent.ACCOUNT, CommonCachingAgent.REGION, CommonCachingAgent.CONTAINER_INSTANCE_ARN_1)
+ def targetHealthKey = Keys.getTargetHealthKey(CommonCachingAgent.ACCOUNT, CommonCachingAgent.REGION, targetGroupArn)
ObjectMapper mapper = new ObjectMapper()
Map<String, Object> loadbalancerMap = mapper.convertValue(new LoadBalancer().withTargetGroupArn(targetGroupArn).withContainerPort(1338), Map.class)
+ Map<String, Object> targetHealthMap = mapper.convertValue(
+ new TargetHealthDescription().withTarget(new TargetDescription().withId(CommonCachingAgent.EC2_INSTANCE_ID_1).withPort(1338)).withTargetHealth(new TargetHealth().withState(TargetHealthStateEnum.Healthy)), Map.class)
+ Map<String, Object> targetHealthMap2 = mapper.convertValue(
+ new TargetHealthDescription().withTarget(new TargetDescription().withId("192.168.0.100").withPort(1338)).withTargetHealth(new TargetHealth().withState(TargetHealthStateEnum.Healthy)), Map.class)
+
+ def targetHealths = new ArrayList<>();
+ targetHealths.add(targetHealthMap)
+ targetHealths.add(targetHealthMap2)
providerCache.filterIdentifiers(_, _) >> []
@@ -86,6 +95,14 @@ class TaskHealthCachingAgentSpec extends Specification {
]
def containerInstanceCache = new DefaultCacheData(containerInstanceKey, containerInstanceAttributes, Collections.emptyMap())
providerCache.get(Keys.Namespace.CONTAINER_INSTANCES.toString(), containerInstanceKey) >> containerInstanceCache
+
+ def targetHealthAttributes = [
+ targetGroupArn : targetGroupArn,
+ targetHealthDescriptions : targetHealths
+ ]
+
+ def targetHealthCache = new DefaultCacheData(targetHealthKey, targetHealthAttributes, Collections.emptyMap())
+ providerCache.get(TARGET_HEALTHS.toString(), targetHealthKey) >> targetHealthCache
}
def 'should get a list of task health'() {
@@ -120,15 +137,6 @@ class TaskHealthCachingAgentSpec extends Specification {
def taskHealthList = agent.getItems(ecs, providerCache)
then:
- amazonloadBalancing.describeTargetHealth({ DescribeTargetHealthRequest request ->
- request.targetGroupArn == targetGroupArn
- request.targets.size() == 1
- request.targets.get(0).id == CommonCachingAgent.EC2_INSTANCE_ID_1
- request.targets.get(0).port == 1338
- }) >> new DescribeTargetHealthResult().withTargetHealthDescriptions(
- new TargetHealthDescription().withTargetHealth(new TargetHealth().withState(TargetHealthStateEnum.Healthy))
- )
-
taskHealthList.size() == 1
TaskHealth taskHealth = taskHealthList.get(0)
taskHealth.getState() == 'Up'
@@ -175,15 +183,6 @@ class TaskHealthCachingAgentSpec extends Specification {
def taskHealthList = agent.getItems(ecs, providerCache)
then:
- amazonloadBalancing.describeTargetHealth({ DescribeTargetHealthRequest request ->
- request.targetGroupArn == targetGroupArn
- request.targets.size() == 1
- request.targets.get(0).id == CommonCachingAgent.EC2_INSTANCE_ID_1
- request.targets.get(0).port == 1338
- }) >> new DescribeTargetHealthResult().withTargetHealthDescriptions(
- new TargetHealthDescription().withTargetHealth(new TargetHealth().withState(TargetHealthStateEnum.Healthy))
- )
-
taskHealthList.size() == 1
TaskHealth taskHealth = taskHealthList.get(0)
taskHealth.getState() == 'Up'
@@ -229,15 +228,6 @@ class TaskHealthCachingAgentSpec extends Specification {
def taskHealthList = agent.getItems(ecs, providerCache)
then:
- amazonloadBalancing.describeTargetHealth({ DescribeTargetHealthRequest request ->
- request.targetGroupArn == targetGroupArn
- request.targets.size() == 1
- request.targets.get(0).id == CommonCachingAgent.EC2_INSTANCE_ID_1
- request.targets.get(0).port == 1338
- }) >> new DescribeTargetHealthResult().withTargetHealthDescriptions(
- new TargetHealthDescription().withTargetHealth(new TargetHealth().withState(TargetHealthStateEnum.Healthy))
- )
-
taskHealthList.size() == 1
TaskHealth taskHealth = taskHealthList.get(0)
taskHealth.getState() == 'Up'
@@ -303,15 +293,6 @@ class TaskHealthCachingAgentSpec extends Specification {
def taskHealthList = agent.getItems(ecs, providerCache)
then:
- amazonloadBalancing.describeTargetHealth({ DescribeTargetHealthRequest request ->
- request.targetGroupArn == targetGroupArn
- request.targets.size() == 1
- request.targets.get(0).id == "192.168.0.100"
- request.targets.get(0).port == 1338
- }) >> new DescribeTargetHealthResult().withTargetHealthDescriptions(
- new TargetHealthDescription().withTargetHealth(new TargetHealth().withState(TargetHealthStateEnum.Healthy))
- )
-
taskHealthList.size() == 1
TaskHealth taskHealth = taskHealthList.get(0)
taskHealth.getState() == 'Up'

original_message:
feat(ecs): Refactor TaskHealthCachingAgent to use TargetHealthCache (#4275)

* feat(ecs): Refactor TaskHealthCachingAgent to use TargetHealthCache

* Determines Task health with cached ALB target healths vs.
calls to Elastic Load Balancing

* Return status_unknown for targets not found