AMBARI-15280. Config version remains is_current=true even after it's group is deleted. (stoader)

ambari-server/src/main/java/org/apache/ambari/server/controller/ServiceConfigVersionResponse.java

@@ -37,6 +37,17 @@ import com.google.inject.Inject;
 
 @StaticallyInject
 public class ServiceConfigVersionResponse {
+  /**
+   * Name used for default config group.
+   */
+  public static final String DEFAULT_CONFIG_GROUP_NAME = "default";
+
+  /**
+   * Name used for config groups that were deleted in the service config version response.
+   */
+  public static final String DELETED_CONFIG_GROUP_NAME = "deleted";
+
+
   @JsonProperty("cluster_name")
   @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
   private final String clusterName;

ambari-server/src/main/java/org/apache/ambari/server/state/cluster/ClusterImpl.java

@@ -2349,11 +2349,11 @@ public class ClusterImpl implements Cluster {
 
     configChangeLog.info("Cluster '{}' changed by: '{}'; service_name='{}' config_group='{}' config_group_id='{}' " +
         "version='{}'", getClusterName(), user, serviceName,
-      configGroup == null ? "default" : configGroup.getName(),
+      configGroup == null ? ServiceConfigVersionResponse.DEFAULT_CONFIG_GROUP_NAME : configGroup.getName(),
       configGroup == null ? "-1" : configGroup.getId(),
       serviceConfigEntity.getVersion());
 
-    String configGroupName = configGroup != null ? configGroup.getName() : "default";
+    String configGroupName = configGroup != null ? configGroup.getName() : ServiceConfigVersionResponse.DEFAULT_CONFIG_GROUP_NAME;
 
     ServiceConfigVersionResponse response = new ServiceConfigVersionResponse(
         serviceConfigEntity, configGroupName);
@@ -2460,7 +2460,8 @@ public class ClusterImpl implements Cluster {
         // the active config within a group
         ServiceConfigVersionResponse activeServiceConfigResponse = activeServiceConfigResponseGroups.get(serviceConfigVersionResponse.getGroupName());
 
-        if (activeServiceConfigResponse == null) {
+        if (activeServiceConfigResponse == null && !ServiceConfigVersionResponse.DELETED_CONFIG_GROUP_NAME.equals(serviceConfigVersionResponse.getGroupName())) {
+          // service config version with deleted group should always be marked is not current
           activeServiceConfigResponseGroups.put(serviceConfigVersionResponse.getGroupName(), serviceConfigVersionResponse);
           activeServiceConfigResponse = serviceConfigVersionResponse;
         }
@@ -2468,12 +2469,14 @@ public class ClusterImpl implements Cluster {
         serviceConfigVersionResponse.setConfigurations(new ArrayList<ConfigurationResponse>());
 
         if (serviceConfigEntity.getGroupId() == null) {
-          if (serviceConfigVersionResponse.getCreateTime() > activeServiceConfigResponse.getCreateTime())
+          if (serviceConfigVersionResponse.getCreateTime() > activeServiceConfigResponse.getCreateTime()) {
             activeServiceConfigResponseGroups.put(serviceConfigVersionResponse.getGroupName(), serviceConfigVersionResponse);
+          }
         }
         else if (clusterConfigGroups != null && clusterConfigGroups.containsKey(serviceConfigEntity.getGroupId())){
-          if (serviceConfigVersionResponse.getVersion() > activeServiceConfigResponse.getVersion())
+          if (serviceConfigVersionResponse.getVersion() > activeServiceConfigResponse.getVersion()) {
             activeServiceConfigResponseGroups.put(serviceConfigVersionResponse.getGroupName(), serviceConfigVersionResponse);
+          }
         }
 
         serviceConfigVersionResponse.setIsCurrent(false);
@@ -2554,10 +2557,10 @@ public class ClusterImpl implements Cluster {
       if (configGroup != null) {
         groupName = configGroup.getName();
       } else {
-        groupName = "deleted";
+        groupName = ServiceConfigVersionResponse.DELETED_CONFIG_GROUP_NAME;
       }
     } else {
-      groupName = "default";
+      groupName = ServiceConfigVersionResponse.DEFAULT_CONFIG_GROUP_NAME;
     }
 
     ServiceConfigVersionResponse serviceConfigVersionResponse = new ServiceConfigVersionResponse(

ambari-server/src/test/java/org/apache/ambari/server/state/cluster/ClusterTest.java

@@ -1404,6 +1404,89 @@ public class ClusterTest {
       is(expectedServiceConfigResponses));
   }
 
+  @Test
+  public void testAllServiceConfigVersionsWithDeletedConfigGroups() throws Exception {
+    // Given
+    createDefaultCluster();
+
+    Config hdfsSiteConfigV1 = configFactory.createNew(c1, "hdfs-site", ImmutableMap.of("p1", "v1"), ImmutableMap.<String, Map<String,String>>of());
+    hdfsSiteConfigV1.setTag("version1");
+    hdfsSiteConfigV1.persist();
+
+    c1.addConfig(hdfsSiteConfigV1);
+
+
+    ServiceConfigVersionResponse hdfsSiteConfigResponseV1 = c1.addDesiredConfig("admin", Collections.singleton(hdfsSiteConfigV1));
+    List<ConfigurationResponse> configResponsesDefaultGroup =  Collections.singletonList(
+      new ConfigurationResponse(c1.getClusterName(), hdfsSiteConfigV1.getStackId(),
+        hdfsSiteConfigV1.getType(), hdfsSiteConfigV1.getTag(), hdfsSiteConfigV1.getVersion(),
+        hdfsSiteConfigV1.getProperties(), hdfsSiteConfigV1.getPropertiesAttributes(), hdfsSiteConfigV1.getPropertiesTypes())
+    );
+
+    hdfsSiteConfigResponseV1.setConfigurations(configResponsesDefaultGroup);
+
+    Config hdfsSiteConfigV2 = configFactory.createNew(c1, "hdfs-site", ImmutableMap.of("p1", "v2"), ImmutableMap.<String, Map<String,String>>of());
+    hdfsSiteConfigV2.setTag("version2");
+
+    ConfigGroup configGroup = configGroupFactory.createNew(c1, "configGroup1", "version1", "test description", ImmutableMap.of(hdfsSiteConfigV2.getType(), hdfsSiteConfigV2), ImmutableMap.<Long, Host>of());
+    configGroup.persist();
+
+    c1.addConfigGroup(configGroup);
+    ServiceConfigVersionResponse hdfsSiteConfigResponseV2 = c1.createServiceConfigVersion("HDFS", "admin", "test note", configGroup);
+    hdfsSiteConfigResponseV2.setConfigurations(Collections.singletonList(
+      new ConfigurationResponse(c1.getClusterName(), hdfsSiteConfigV2.getStackId(),
+        hdfsSiteConfigV2.getType(), hdfsSiteConfigV2.getTag(), hdfsSiteConfigV2.getVersion(),
+        hdfsSiteConfigV2.getProperties(), hdfsSiteConfigV2.getPropertiesAttributes(), hdfsSiteConfigV2.getPropertiesTypes())
+    ));
+
+    // delete the config group
+    c1.deleteConfigGroup(configGroup.getId());
+
+
+    // hdfs config v3
+    ServiceConfigVersionResponse hdfsSiteConfigResponseV3 = c1.createServiceConfigVersion("HDFS", "admin", "new config in default group", null);
+    hdfsSiteConfigResponseV3.setConfigurations(configResponsesDefaultGroup);
+    hdfsSiteConfigResponseV3.setIsCurrent(true); // this is the active config in default config group as it's more recent than V1
+
+
+
+    // When
+
+    List<ServiceConfigVersionResponse> allServiceConfigResponses = c1.getServiceConfigVersions();
+
+    Collections.sort(
+      allServiceConfigResponses,
+      new Comparator<ServiceConfigVersionResponse>() {
+        @Override
+        public int compare(ServiceConfigVersionResponse o1, ServiceConfigVersionResponse o2) {
+          return o1.getVersion().compareTo(o2.getVersion());
+        }
+      }
+    );
+
+
+    // Then
+
+    assertEquals(3, allServiceConfigResponses.size());
+
+    // all configs that was created as member of config group 'configGroup1' should be marked as 'not current'
+    // as the parent config group has been deleted
+
+    // default group
+    assertEquals(false, allServiceConfigResponses.get(0).getIsCurrent());
+    assertEquals(ServiceConfigVersionResponse.DEFAULT_CONFIG_GROUP_NAME, allServiceConfigResponses.get(0).getGroupName());
+
+    assertEquals(true, allServiceConfigResponses.get(2).getIsCurrent());
+    assertEquals(ServiceConfigVersionResponse.DEFAULT_CONFIG_GROUP_NAME, allServiceConfigResponses.get(2).getGroupName());
+
+    // deleted group
+    assertEquals(false, allServiceConfigResponses.get(1).getIsCurrent());
+    assertEquals(ServiceConfigVersionResponse.DELETED_CONFIG_GROUP_NAME, allServiceConfigResponses.get(1).getGroupName());
+
+
+
+  }
+
   @Test
   public void testTransitionClusterVersion() throws Exception {
     createDefaultCluster();