feat(aws): Remove support for the legacy 'v1' reservation report (#5045)

This also drops support for vpc/non-vpc differentiation.
spinnaker · Oct 26, 2020 · edf2bf8 · edf2bf8
1 parent f9dd07d
commit edf2bf8
Show file tree

Hide file tree

Showing 4 changed files with 3 additions and 111 deletions.
diff --git a/...com/netflix/spinnaker/clouddriver/aws/provider/agent/ReservationReportCachingAgent.groovy b/...com/netflix/spinnaker/clouddriver/aws/provider/agent/ReservationReportCachingAgent.groovy
@@ -85,7 +85,6 @@ class ReservationReportCachingAgent implements CachingAgent, CustomScheduledAgen
   final AccountReservationDetailSerializer accountReservationDetailSerializer
   final MetricsSupport metricsSupport
   final Registry registry
-  final Map<String, Boolean> vpcOnlyAccounts = new ConcurrentHashMap<>()
 
 
   ReservationReportCachingAgent(Registry registry,
@@ -193,15 +192,6 @@ class ReservationReportCachingAgent implements CachingAgent, CustomScheduledAgen
       !errorsByRegion.containsKey(it.region())
     }
 
-    // v1 is a legacy report that does not differentiate between vpc and non-vpc reserved instances
-    accountReservationDetailSerializer.mergeVpcReservations = true
-    def v1 = objectMapper.readValue(
-      objectMapper
-        .writerWithView(AmazonReservationReport.Views.V1.class)
-        .writeValueAsString(amazonReservationReport),
-      Map
-    )
-
     // v2 differentiates reservations between vpc and non-vpc
     accountReservationDetailSerializer.mergeVpcReservations = false
     def v2 = objectMapper.readValue(
@@ -244,7 +234,6 @@ class ReservationReportCachingAgent implements CachingAgent, CustomScheduledAgen
 
     return new DefaultCacheResult(
       (RESERVATION_REPORTS.ns): [
-        new MutableCacheData("v1", ["report": v1], [:]),
         new MutableCacheData("v2", ["report": v2], [:]),
 
         // temporarily backport the changes from v4 to v3 (leaving v2_5 to be what 'v3' used to be)
@@ -301,11 +290,7 @@ class ReservationReportCachingAgent implements CachingAgent, CustomScheduledAgen
             def osType = operatingSystemType(it.productDescription)
             def reservation = getReservation(region.name, it.availabilityZone, osType.name, it.instanceType)
             reservation.totalReserved.addAndGet(it.instanceCount)
-            if (osType.isVpc || vpcOnlyAccounts.get(credentials.getName())) {
-              reservation.getAccount(credentials.name).reservedVpc.addAndGet(it.instanceCount)
-            } else {
-              reservation.getAccount(credentials.name).reserved.addAndGet(it.instanceCount)
-            }
+            reservation.getAccount(credentials.name).reservedVpc.addAndGet(it.instanceCount)
           }
 
           startTime = System.currentTimeMillis()
@@ -414,18 +399,6 @@ class ReservationReportCachingAgent implements CachingAgent, CustomScheduledAgen
     this.cacheView
   }
 
-  public void addVPCOnlyAccounts(String accountName, Boolean vpcOnly) {
-    vpcOnlyAccounts.put(accountName, vpcOnly)
-  }
-
-  public void deleteVPCOnlyAccounts(String account) {
-    vpcOnlyAccounts.remove(account)
-  }
-
-  protected getVPCOnlyaccounts() {
-    return Collections.unmodifiableMap(vpcOnlyAccounts)
-  }
-
   static class AccountReservationDetailSerializer extends JsonSerializer<AmazonReservationReport.AccountReservationDetail> {
     ObjectMapper objectMapper = new ObjectMapper()
     boolean mergeVpcReservations

diff --git a/...ovy/com/netflix/spinnaker/clouddriver/aws/security/AmazonCredentialsLifecycleHandler.java b/...ovy/com/netflix/spinnaker/clouddriver/aws/security/AmazonCredentialsLifecycleHandler.java
@@ -17,9 +17,6 @@
 
 package com.netflix.spinnaker.clouddriver.aws.security;
 
-import com.amazonaws.services.ec2.AmazonEC2;
-import com.amazonaws.services.ec2.model.DescribeAccountAttributesRequest;
-import com.amazonaws.services.ec2.model.DescribeAccountAttributesResult;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.netflix.spectator.api.Registry;
 import com.netflix.spinnaker.cats.agent.Agent;
@@ -88,9 +85,6 @@ public class AmazonCredentialsLifecycleHandler
   public void credentialsAdded(@NotNull NetflixAmazonCredentials credentials) {
     scheduleAgents(credentials);
     scheduleReservationReportCachingAgent();
-    if (reservationReportCachingAgentScheduled) {
-      addVPCOnlyAccountMapping(credentials);
-    }
   }
 
   @Override
@@ -103,9 +97,6 @@ public void credentialsUpdated(@NotNull NetflixAmazonCredentials credentials) {
   public void credentialsDeleted(@NotNull NetflixAmazonCredentials credentials) {
     replaceCurrentImageCachingAgent(credentials);
     unscheduleAgents(credentials);
-    if (reservationReportCachingAgentScheduled) {
-      deleteVPCOnlyAccountMapping(credentials);
-    }
   }
 
   private void replaceCurrentImageCachingAgent(NetflixAmazonCredentials credentials) {
@@ -158,9 +149,6 @@ private void scheduleAgents(NetflixAmazonCredentials credentials) {
     scheduleAWSProviderAgents(credentials);
     scheduleAwsInfrastructureProviderAgents(credentials);
     scheduleAwsCleanupAgents(credentials);
-    if (reservationReportCachingAgentScheduled) {
-      addVPCOnlyAccountMapping(credentials);
-    }
   }
 
   private void scheduleAwsInfrastructureProviderAgents(NetflixAmazonCredentials credentials) {
@@ -236,38 +224,4 @@ private void scheduleReservationReportCachingAgent() {
       reservationReportCachingAgentScheduled = true;
     }
   }
-
-  private void addVPCOnlyAccountMapping(NetflixAmazonCredentials credentials) {
-    ReservationReportCachingAgent reservationReportCachingAgent =
-        awsProvider.getAgents().stream()
-            .filter(agent -> agent instanceof ReservationReportCachingAgent)
-            .map(agent -> (ReservationReportCachingAgent) agent)
-            .findFirst()
-            .orElse(null);
-    if (reservationReportCachingAgent != null) {
-      AmazonEC2 amazonEC2 =
-          amazonClientProvider.getAmazonEC2(credentials, credentials.getRegions().get(0).getName());
-      DescribeAccountAttributesResult describeAccountAttributesResult =
-          amazonEC2.describeAccountAttributes(
-              new DescribeAccountAttributesRequest().withAttributeNames("supported-platforms"));
-      reservationReportCachingAgent.addVPCOnlyAccounts(
-          credentials.getName(),
-          describeAccountAttributesResult
-              .getAccountAttributes()
-              .get(0)
-              .getAttributeValues()
-              .stream()
-              .allMatch(attribute -> "VPC".equals(attribute.getAttributeValue())));
-    }
-  }
-
-  private void deleteVPCOnlyAccountMapping(NetflixAmazonCredentials credentials) {
-    awsProvider.getAgents().stream()
-        .filter(agent -> agent instanceof ReservationReportCachingAgent)
-        .map(agent -> (ReservationReportCachingAgent) agent)
-        .findFirst()
-        .ifPresent(
-            reservationReportCachingAgent ->
-                reservationReportCachingAgent.deleteVPCOnlyAccounts(credentials.getName()));
-  }
 }
diff --git a/...m/netflix/spinnaker/clouddriver/aws/security/AmazonCredentialsLifecycleHandlerSpec.groovy b/...m/netflix/spinnaker/clouddriver/aws/security/AmazonCredentialsLifecycleHandlerSpec.groovy
@@ -106,12 +106,7 @@ class AmazonCredentialsLifecycleHandlerSpec extends Specification {
   }
 
   def 'it should add agents'() {
-    def amazonEC2 = Mock(AmazonEC2) {
-      describeAccountAttributes(_) >> new DescribeAccountAttributesResult().withAccountAttributes(
-          new AccountAttribute().withAttributeName("supported-platforms").withAttributeValues(
-            new AccountAttributeValue().withAttributeValue("VPC")
-          ))
-    }
+    def amazonEC2 = Mock(AmazonEC2)
     def amazonClientProvider = Mock(AmazonClientProvider) {
       getAmazonEC2(_, _) >> amazonEC2
     }
@@ -133,35 +128,6 @@ class AmazonCredentialsLifecycleHandlerSpec extends Specification {
       .filter({ agent -> agent instanceof ReservationReportCachingAgent })
       .map({ agent -> (ReservationReportCachingAgent) agent })
       .findFirst().get()
-    reservationReportCachingAgent.getVpcOnlyAccounts().get("three")
-  }
-
-  def 'account should be flagged as not vpc only'() {
-    def amazonEC2 = Mock(AmazonEC2) {
-      describeAccountAttributes(_) >> new DescribeAccountAttributesResult().withAccountAttributes(
-        new AccountAttribute().withAttributeName("supported-platforms").withAttributeValues(
-          new AccountAttributeValue().withAttributeValue("VPC"),
-          new AccountAttributeValue().withAttributeValue("classic")
-        ))
-    }
-    def amazonClientProvider = Mock(AmazonClientProvider) {
-      getAmazonEC2(_, _) >> amazonEC2
-    }
-    def handler = new AmazonCredentialsLifecycleHandler(awsCleanupProvider, awsInfrastructureProvider, awsProvider,
-      amazonCloudProvider, amazonClientProvider, null, null, objectMapper, null, eddaApiFactory, null, registry, reservationReportPool, agentProviders, null, dynamicConfigService, deployDefaults,
-      credentialsRepository)
-    def credThree = TestCredential.named('three')
-
-    when:
-    handler.credentialsAdded(credThree)
-
-    then:
-    handler.reservationReportCachingAgentScheduled
-    def reservationReportCachingAgent = awsProvider.getAgents().stream()
-      .filter({ agent -> agent instanceof ReservationReportCachingAgent })
-      .map({ agent -> (ReservationReportCachingAgent) agent })
-      .findFirst().get()
-    !reservationReportCachingAgent.getVpcOnlyAccounts().get("three")
   }
 
   def 'subsequent call should not add reservation caching agents'() {
@@ -205,6 +171,5 @@ class AmazonCredentialsLifecycleHandlerSpec extends Specification {
       .filter({ agent -> agent instanceof ReservationReportCachingAgent })
       .map({ agent -> (ReservationReportCachingAgent) agent })
       .findFirst().get()
-    !reservationReportCachingAgent.getVpcOnlyAccounts().get("three")
   }
 }
diff --git a/...n/groovy/com/netflix/spinnaker/clouddriver/controllers/ReservationReportController.groovy b/...n/groovy/com/netflix/spinnaker/clouddriver/controllers/ReservationReportController.groovy
@@ -33,7 +33,7 @@ class ReservationReportController {
 
   @RequestMapping(method = RequestMethod.GET)
   Collection<ReservationReport> getReservationReports(@RequestParam Map<String, String> filters) {
-    return getReservationReportsByName("v1", filters)
+    return getReservationReportsByName("v3", filters)
   }
 
   @RequestMapping(method = RequestMethod.GET, value = "/{name}")