From 9362a1cfaed1a6bf57f724324f8bdd9acc752ca4 Mon Sep 17 00:00:00 2001 From: Steve Jones Date: Thu, 10 Jan 2013 16:35:03 -0800 Subject: [PATCH] Add filtering on resource tags for instances (EUCA-4655) Instance filtering using tags is now supported via the new VmInstanceFilterSupport class. The VmInstanceFilterSupportTest provides unit test coverage of the tag filtering configuration. VmControl is updated to use filters when describing instances. VmInstances is updated to allow use of a Criteria / alias Map for instance lookup (for database filters) --- .../main/java/com/eucalyptus/tags/Filter.java | 22 ++++-- .../com/eucalyptus/tags/FilterSupport.java | 2 +- .../java/com/eucalyptus/tags/Filters.java | 1 - .../java/com/eucalyptus/vm/VmControl.java | 14 +++- .../java/com/eucalyptus/vm/VmInstances.java | 72 +++++++++++++------ .../vm/VmInstanceFilterSupport.groovy | 15 ++++ 6 files changed, 96 insertions(+), 30 deletions(-) create mode 100644 clc/modules/cluster-manager/src/test/java/com/eucalyptus/vm/VmInstanceFilterSupport.groovy diff --git a/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/Filter.java b/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/Filter.java index 1e0ec26af7f..0733e49670d 100644 --- a/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/Filter.java +++ b/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/Filter.java @@ -38,23 +38,28 @@ public class Filter { @Nonnull private final Map aliases; @Nonnull private final Criterion criterion; @Nonnull private final Predicate predicate; + private final boolean filteringOnTags; Filter( @Nonnull final Map aliases, @Nonnull final Criterion criterion, - @Nonnull final Predicate predicate ) { + @Nonnull final Predicate predicate, + final boolean filteringOnTags ) { this.aliases = aliases; this.criterion = criterion; this.predicate = predicate; + this.filteringOnTags = filteringOnTags; } - Filter( @Nonnull final Predicate predicate ) { + Filter( @Nonnull final Predicate predicate, + final boolean filteringOnTags ) { this( Collections.emptyMap(), Restrictions.conjunction(), - predicate ); + predicate, + filteringOnTags ); } private Filter() { - this( Predicates.alwaysTrue() ); + this( Predicates.alwaysTrue(), false ); } /** @@ -87,6 +92,15 @@ public Predicate asPredicate() { return predicate; } + /** + * Does the filter use tags? + * + * @return True if the filter uses any tags + */ + public boolean isFilteringOnTags() { + return filteringOnTags; + } + /** * Create a Filter that will always pass (filters out nothing) */ diff --git a/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/FilterSupport.java b/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/FilterSupport.java index cf0cb38a5f6..ce047bd2b68 100644 --- a/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/FilterSupport.java +++ b/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/FilterSupport.java @@ -550,7 +550,7 @@ public Filter generate( final Map> filters, } if ( tagPresent ) conjunction.add( tagCriterion( accountId, tagConjunction ) ); - return new Filter( aliases, conjunction, Predicates.and( and ) ); + return new Filter( aliases, conjunction, Predicates.and( and ), tagPresent ); } public static FilterSupport forResource( @Nonnull final Class metadataClass ) { diff --git a/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/Filters.java b/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/Filters.java index 78870e40c2e..5dc33d9bd81 100644 --- a/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/Filters.java +++ b/clc/modules/cluster-manager/src/main/java/com/eucalyptus/tags/Filters.java @@ -22,7 +22,6 @@ import java.util.Map; import java.util.Set; import javax.annotation.Nonnull; -import javax.annotation.Nullable; import com.eucalyptus.cloud.CloudMetadata; import com.google.common.base.Function; import com.google.common.collect.ImmutableMap; diff --git a/clc/modules/cluster-manager/src/main/java/com/eucalyptus/vm/VmControl.java b/clc/modules/cluster-manager/src/main/java/com/eucalyptus/vm/VmControl.java index c73c54ccb7f..faff3291bac 100644 --- a/clc/modules/cluster-manager/src/main/java/com/eucalyptus/vm/VmControl.java +++ b/clc/modules/cluster-manager/src/main/java/com/eucalyptus/vm/VmControl.java @@ -99,6 +99,8 @@ import com.eucalyptus.records.EventRecord; import com.eucalyptus.records.EventType; import com.eucalyptus.records.Logs; +import com.eucalyptus.tags.Filter; +import com.eucalyptus.tags.Filters; import com.eucalyptus.util.EucalyptusCloudException; import com.eucalyptus.util.Exceptions; import com.eucalyptus.util.OwnerFullName; @@ -193,13 +195,19 @@ public DescribeInstancesResponseType describeInstances( final DescribeInstancesT boolean showAll = msg.getInstancesSet( ).remove( "verbose" ); final ArrayList instancesSet = msg.getInstancesSet( ); final Multimap instanceMap = TreeMultimap.create( ); - final Map reservations = Maps.newHashMap( ); - Predicate filter = CloudMetadatas.filterPrivilegesById( msg.getInstancesSet( ) ); + final Map reservations = Maps.newHashMap(); + final Filter filter = Filters.generate( msg.getFilterSet(), VmInstance.class ); + final Predicate requestedAndAccessible = CloudMetadatas.filteringFor( VmInstance.class ) + .byId( msg.getInstancesSet( ) ) + .byPredicate( filter.asPredicate() ) + .byPredicate( filter.isFilteringOnTags() ? Predicates.not( VmState.TERMINATED ) : Predicates.alwaysTrue() ) // terminated instances have no tags + .byPrivileges() + .buildPredicate(); OwnerFullName ownerFullName = ( ctx.hasAdministrativePrivileges( ) && showAll ) ? null : ctx.getUserFullName( ).asAccountFullName( ); try { - for ( final VmInstance vm : VmInstances.list( ownerFullName, filter ) ) { + for ( final VmInstance vm : VmInstances.list( ownerFullName, filter.asCriterion(), filter.getAliases(), requestedAndAccessible ) ) { if ( !instancesSet.isEmpty( ) && !instancesSet.contains( vm.getInstanceId( ) ) ) { continue; } diff --git a/clc/modules/cluster-manager/src/main/java/com/eucalyptus/vm/VmInstances.java b/clc/modules/cluster-manager/src/main/java/com/eucalyptus/vm/VmInstances.java index db560002d55..f2ba0970c38 100644 --- a/clc/modules/cluster-manager/src/main/java/com/eucalyptus/vm/VmInstances.java +++ b/clc/modules/cluster-manager/src/main/java/com/eucalyptus/vm/VmInstances.java @@ -75,8 +75,11 @@ import java.util.NoSuchElementException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import javax.persistence.EntityTransaction; import org.apache.log4j.Logger; +import org.hibernate.criterion.Criterion; import org.hibernate.criterion.Example; import org.hibernate.criterion.MatchMode; import org.hibernate.criterion.Projections; @@ -112,6 +115,7 @@ import com.eucalyptus.records.EventType; import com.eucalyptus.records.Logs; import com.eucalyptus.reporting.event.ResourceAvailabilityEvent; +import com.eucalyptus.tags.FilterSupport; import com.eucalyptus.util.Callback; import com.eucalyptus.util.HasNaturalId; import com.eucalyptus.util.LogUtil; @@ -129,6 +133,8 @@ import com.google.common.base.Function; import com.google.common.base.Functions; import com.google.common.base.Predicate; +import com.google.common.base.Predicates; +import com.google.common.base.Supplier; import com.google.common.collect.Collections2; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -654,30 +660,56 @@ public static List list( ) { return list( null ); } - public static List list( Predicate predicate ) { + public static List list( @Nullable Predicate predicate ) { return list( null, null, predicate ); } - public static List list( OwnerFullName ownerFullName, Predicate predicate ) { + public static List list( @Nullable OwnerFullName ownerFullName, + @Nullable Predicate predicate ) { return list( ownerFullName, null, predicate ); } - - public static List list( String instanceId, Predicate predicate ) { + + public static List list( @Nullable final OwnerFullName ownerFullName, + final Criterion criterion, + final Map aliases, + @Nullable final Predicate predicate ) { + return list( new Supplier>() { + @Override + public List get() { + return Entities.query( VmInstance.named( ownerFullName, null ), false, criterion, aliases ); + } + }, predicate ); + } + + public static List list( @Nullable String instanceId, + @Nullable Predicate predicate ) { return list( null, instanceId, predicate ); } - public static List list( OwnerFullName ownerFullName, String instanceId, Predicate predicate ) { + public static List list( @Nullable final OwnerFullName ownerFullName, + @Nullable final String instanceId, + @Nullable Predicate predicate ) { + return list( new Supplier>() { + @Override + public List get() { + return Entities.query( VmInstance.named( ownerFullName, instanceId ) ); + } + }, predicate ); + } + + private static List list( @Nonnull Supplier> instancesSupplier, + @Nullable Predicate predicate ) { predicate = checkPredicate( predicate ); - List ret = listPersistent( ownerFullName, instanceId, predicate ); + List ret = listPersistent( instancesSupplier, predicate ); ret.addAll( Collections2.filter( terminateCache.values( ), predicate ) ); return ret; } - - private static List listPersistent( OwnerFullName ownerFullName, String instanceId, Predicate predicate ) { - predicate = checkPredicate( predicate ); + + private static List listPersistent( @Nonnull Supplier> instancesSupplier, + @Nonnull Predicate predicate ) { final EntityTransaction db = Entities.get( VmInstance.class ); try { - final Iterable vms = Iterables.filter( Entities.query( VmInstance.named( ownerFullName, instanceId ) ), predicate ); + final Iterable vms = Iterables.filter( instancesSupplier.get(), predicate ); db.commit( ); return Lists.newArrayList( vms ); } catch ( final Exception ex ) { @@ -688,17 +720,10 @@ private static List listPersistent( OwnerFullName ownerFullName, Str } } - private static Predicate checkPredicate( Predicate predicate ) { - if ( predicate == null ) { - predicate = new Predicate( ) { - - @Override - public boolean apply( VmInstance input ) { - return true; - } - }; - } - return predicate; + private static Predicate checkPredicate( Predicate predicate ) { + return predicate == null ? + Predicates.alwaysTrue() : + predicate; } public static boolean contains( final String name ) { @@ -852,4 +877,9 @@ public void fireEvent( final ClockTick event ) { } } + public static class VmInstanceFilterSupport extends FilterSupport { + public VmInstanceFilterSupport() { + super( builderFor( VmInstance.class ).withTagFiltering( VmInstanceTag.class, "instance" ) ); + } + } } diff --git a/clc/modules/cluster-manager/src/test/java/com/eucalyptus/vm/VmInstanceFilterSupport.groovy b/clc/modules/cluster-manager/src/test/java/com/eucalyptus/vm/VmInstanceFilterSupport.groovy new file mode 100644 index 00000000000..41becfc3e2f --- /dev/null +++ b/clc/modules/cluster-manager/src/test/java/com/eucalyptus/vm/VmInstanceFilterSupport.groovy @@ -0,0 +1,15 @@ +package com.eucalyptus.vm + +import org.junit.Test +import com.eucalyptus.tags.FilterSupportTest + +/** + * Unit tests for instance filter support + */ +class VmInstanceFilterSupport extends FilterSupportTest.InstanceTest { + + @Test + void testFilteringSupport() { + assertValid( new VmInstances.VmInstanceFilterSupport() ) + } +}