(ARISTA-21) Use snmp_community flush for management

Without this patch the snmp_community provider does not have the ability
to destroy resources on the target device, nor the ability to manage a
specific property of an existing resource.

This patch addresses the problem by implementing the provider flush
method.  The behavior of all the property setter methods is to set a key
for the property in the @property_flush instance variable.  The flush
method merges the @property_flush instance variable on top of the
@property_hash variable in order to preserve existing properties that
are not being managed by the applied catalog.

lib/puppet/provider/snmp_community/eos.rb

@@ -30,22 +30,35 @@ def exists?
   end
 
   def create
-    allowed_keys = [:name, :group, :acl]
-    property_hash = resource.to_hash.select do |key, _|
-      allowed_keys.include? key
+    @property_flush = resource.to_hash.select do |key, _|
+      [:name, :group, :acl].include? key
     end
-    api.snmp_community_create(property_hash)
-    @property_hash = property_hash
-    @property_hash[:ensure] = :present
+    @property_flush[:ensure] = :present
+  end
+
+  def destroy
+    @property_flush = { name: name, ensure: :absent }
   end
 
   def group=(value)
-    fail NotImplementedError, 'not implemented'
-    @property_hash[:group] = value
+    @property_flush[:group] = value
   end
 
   def acl=(value)
-    fail NotImplementedError, 'not implemented'
-    @property_hash[:acl] = value
+    @property_flush[:acl] = value
+  end
+
+  def flush
+    new_property_hash = @property_hash.merge(@property_flush)
+    new_property_hash[:name] = name
+
+    case new_property_hash[:ensure]
+    when :absent, 'absent'
+      api.snmp_community_destroy(name: name)
+    else
+      api.snmp_community_set(new_property_hash)
+    end
+
+    @property_hash = new_property_hash
   end
 end

lib/puppet_x/net_dev/eos_api/snmp_methods.rb

@@ -212,30 +212,46 @@ def map_snmp_keys(key, val)
           end
         end
         private :map_snmp_keys
-      end
 
-      ##
-      # snmp_community_create creates a new snmp community on the target device
-      # given a hash of attributes from the resource module.
-      #
-      # @option opts [String] :name ('public') The community name
-      #
-      # @option opts [Symbol] :group (:ro) :ro or :rw for read-only or
-      #   read-write access control for the community name.
-      #
-      # @option opts [String] :acl ('stest1') The standard ACL name defined on
-      #   the switch.  This ACL is defined using the `ip access-list standard
-      #   stest1` command.
-      #
-      # @api public
-      #
-      # @return [Boolean] true if the resource was successfully created
-      def snmp_community_create(opts)
-        prefix = %w(enable configure)
-        cmd = "snmp-server community #{opts[:name]}"
-        cmd << " #{opts[:group]}" if opts[:group]
-        cmd << " #{opts[:acl]}" if opts[:acl]
-        eapi_action([*prefix, cmd], 'create snmp community') && true || false
+        ##
+        # snmp_community_set creates or updates an snmp community on the target
+        # device given a hash of attributes from the resource model.
+        #
+        # @option opts [String] :name ('public') The community name
+        #
+        # @option opts [Symbol] :group (:ro) :ro or :rw for read-only or
+        #   read-write access control for the community name.
+        #
+        # @option opts [String] :acl ('stest1') The standard ACL name defined on
+        #   the switch.  This ACL is defined using the `ip access-list standard
+        #   stest1` command.
+        #
+        # @api public
+        #
+        # @return [Boolean] true if the resource was successfully created
+        def snmp_community_set(opts)
+          prefix = %w(enable configure)
+          cmd = "snmp-server community #{opts[:name]}"
+          cmd << " #{opts[:group]}" if opts[:group]
+          cmd << " #{opts[:acl]}" if opts[:acl]
+          eapi_action([*prefix, cmd], 'define snmp community') && true || false
+        end
+
+        ##
+        # snmp_community_destroy deletes an SNMP community from the target
+        # device.  given a hash of attributes from the resource model.
+        #
+        # @option opts [String] :name ('public') The community name
+        #
+        # @api public
+        #
+        # @return [Boolean] true if the resource was successfully created
+        def snmp_community_destroy(opts)
+          prefix = %w(enable configure)
+          cmd = "no snmp-server community #{opts[:name]}"
+          result = eapi_action([*prefix, cmd], 'destroy snmp community')
+          result && true || false
+        end
       end
     end
   end

spec/unit/puppet/provider/snmp_community/eos_spec.rb

@@ -19,7 +19,6 @@
     }.merge(resource_override)
   end
 
-
   let(:resource) { type.new(resource_hash) }
   let(:provider) { described_class.new(resource) }
 
@@ -41,38 +40,147 @@
     end
   end
 
+  describe '#flush' do
+    before :each do
+      allow(provider.api).to receive(:snmp_community_set)
+      allow(provider.api).to receive(:snmp_community_destroy)
+    end
+
+    context 'after create' do
+      subject do
+        provider.create
+        provider.flush
+      end
+
+      it 'calls api.snmp_community_set' do
+        expected_args = {
+          name: 'public',
+          group: :ro,
+          acl: 'Public Community',
+          ensure: :present
+        }
+        expect(provider.api).to receive(:snmp_community_set)
+          .with(expected_args)
+        subject
+      end
+
+      it 'sets the ensure value to :present' do
+        subject
+        expect(provider.ensure).to eq(:present)
+      end
+
+      it 'sets the group to :ro' do
+        subject
+        expect(provider.group).to eq :ro
+      end
+
+      it 'sets the acl to "Public Community"' do
+        subject
+        expect(provider.acl).to eq 'Public Community'
+      end
+    end
+
+    context 'after destroy' do
+      subject do
+        provider.destroy
+        provider.flush
+      end
+
+      it 'calls api.snmp_community_set' do
+        expect(provider.api).to receive(:snmp_community_destroy)
+          .with(name: 'public')
+        subject
+      end
+
+      it 'sets the ensure value to :absent' do
+        subject
+        expect(provider.ensure).to eq(:absent)
+      end
+    end
+
+    context 'when changing the group' do
+      let(:provider) { described_class.new(resource_hash) }
+      let :expected_args do
+        resource_hash.merge(group: 'rw', acl: 'Public Community')
+      end
+
+      it 'does not unset the acl' do
+        provider.group = 'rw'
+        expect(provider.api).to receive(:snmp_community_set)
+          .with(expected_args)
+        provider.flush
+      end
+    end
+  end
+
   describe '#create' do
     subject { provider.create }
-    before :each do
-      allow(provider.api).to receive(:snmp_community_create)
+
+    it 'sets @property_flush with ensure: present' do
+      subject
+      expect(provider.instance_variable_get(:@property_flush))
+        .to include(ensure: :present)
     end
 
-    it 'calls api.snmp_community_create' do
-      expect(provider.api).to receive(:snmp_community_create)
-        .with(name: 'public', group: :ro, acl: 'Public Community')
+    it 'sets @property_flush with group: :ro' do
       subject
+      expect(provider.instance_variable_get(:@property_flush))
+        .to include(group: :ro)
     end
 
-    it 'sets the ensure value to :present' do
+    it 'sets @property_flush with acl: "Public Community"' do
       subject
-      expect(provider.ensure).to eq :present
+      expect(provider.instance_variable_get(:@property_flush))
+        .to include(acl: 'Public Community')
     end
 
-    it 'sets the group to :ro' do
+    it 'sets @property_flush with name: "public"' do
       subject
-      expect(provider.group).to eq :ro
+      expect(provider.instance_variable_get(:@property_flush))
+        .to include(name: 'public')
     end
+  end
+
+  describe '#destroy' do
+    subject { provider.destroy }
 
-    it 'sets the acl to "Public Community"' do
+    it 'sets @property_flush with ensure: absent' do
       subject
-      expect(provider.acl).to eq 'Public Community'
+      expect(provider.instance_variable_get(:@property_flush))
+        .to include(ensure: :absent)
     end
   end
 
   describe '#group=(value)' do
+    subject { provider.group = val }
+
+    [:ro, :rw].each do |value|
+      context "#group = #{value.inspect}" do
+        let(:val) { value }
+
+        it "sets @property_flush[:group] = #{value.inspect}" do
+          subject
+          expect(provider.instance_variable_get(:@property_flush))
+            .to include(group: value)
+        end
+      end
+    end
   end
 
   describe '#acl=(value)' do
+    subject { provider.acl = val }
+
+    %w(stest1 stest2 foo).each do |value|
+      context "#acl = #{value.inspect}" do
+        let(:val) { value }
+
+        it "sets @property_flush[:acl] = #{value.inspect}" do
+          subject
+          expect(provider.instance_variable_get(:@property_flush))
+            .to include(acl: value)
+        end
+      end
+    end
   end
 
   describe '#exists?' do

spec/unit/puppet_x/net_dev/eos_api/snmp_methods_spec.rb

@@ -243,8 +243,8 @@
     end
   end
 
-  describe '#snmp_community_create' do
-    subject { api.snmp_community_create(resource_hash) }
+  describe '#snmp_community_set' do
+    subject { api.snmp_community_set(resource_hash) }
     let(:prefix) { %w(enable configure) }
 
     context 'when the api call succeeds' do
@@ -267,7 +267,7 @@
       it 'sets the group and the acl in that positional order' do
         expected = 'snmp-server community public ro stest1'
         expect(api).to receive(:eapi_action)
-          .with([*prefix, expected], 'create snmp community')
+          .with([*prefix, expected], 'define snmp community')
         subject
       end
     end
@@ -280,7 +280,7 @@
       it 'sets the group and not the acl' do
         expected = 'snmp-server community public ro'
         expect(api).to receive(:eapi_action)
-          .with([*prefix, expected], 'create snmp community')
+          .with([*prefix, expected], 'define snmp community')
         subject
       end
     end
@@ -293,7 +293,36 @@
       it 'sets the acl and not the group' do
         expected = 'snmp-server community public stest1'
         expect(api).to receive(:eapi_action)
-          .with([*prefix, expected], 'create snmp community')
+          .with([*prefix, expected], 'define snmp community')
+        subject
+      end
+    end
+  end
+
+  describe '#snmp_community_destroy' do
+    subject { api.snmp_community_destroy(resource_hash) }
+    let(:prefix) { %w(enable configure) }
+
+    let :resource_hash do
+      { name: 'public' }
+    end
+
+    context 'when the api call succeeds' do
+      before :each do
+        allow(api).to receive(:eapi_action).and_return(true)
+      end
+
+      it { is_expected.to eq(true) }
+    end
+
+    describe 'expected REST API call' do
+      it 'calls eapi_action with []' do
+        expected = [[*prefix, 'no snmp-server community public'],
+                    'destroy snmp community']
+        expect(api).to receive(:eapi_action)
+          .with(*expected)
+          .and_return(true)
+
         subject
       end
     end