create, describe, and destroy elastic file systems #304

ehowe · 2016-10-05T18:29:45Z

No description provided.

lanej · 2016-10-05T22:00:10Z

lib/fog/aws/efs.rb

+          raise if match.empty?
+          raise case match[:message]
+                when /invalid file system id/i
+                  Fog::AWS::EFS::NotFound.slurp(error, match[:message])


note: NotFound is an inherited constant from Fog::Service

lanej · 2016-10-05T22:01:44Z

lib/fog/aws/requests/efs/delete_file_system.rb

+            raise Fog::AWS::EFS::NotFound.new("invalid file system ID: #{id}")
+          end
+
+          if file_system["NumberOfMountTargets"] > 0


can I get a test for this condition ? Only code path that exercises the FileSystemInUse constant.

Unfortunately I can't. I don't have access to an account with an appropriate configuration to test this in real mode. There is also no support here for mounting a file system or creating a mount target for the same reason. I mocked it purely based on the API documentation. Would you rather I pull this out, or write a test that just manipulates the mock data to test that if statement?

i ran these tests against a live account without failure.

i'd settle for a mock-only test that manipulates the underlying data store test to flex the conditional.

I'll add that spec, but I'm going to try to add more functionality to this PR before getting it merged. I think I figured out how to create mount targets, so it might not need to be a mock only spec.

ok. lmk when you're ready

lanej

approved accidentally. still getting the hang of these reviews.

lanej · 2016-10-06T23:29:07Z

lib/fog/aws/efs.rb

@@ -55,7 +60,7 @@ def initialize(options={})
          @instrumentor       = options[:instrumentor]
          @instrumentor_name  = options[:instrumentor_name] || 'fog.aws.efs'

-          @region     = 'us-east-1'
+          @region     = options[:region]     || 'us-east-1'


ehowe · 2016-10-07T14:01:59Z

@lanej I think I'm done with this. Tests pass for realsies against a live account.

lanej

waiting for feedback on a test failure.

lanej · 2016-10-07T16:44:19Z

tests/models/efs/mount_target_tests.rb

+    end
+
+    tests("#security_groups=") do
+      @instance.security_groups = []


I got:

AWS::EFS | mount target (aws, efs) success #save + succeeds #security_groups + returns ["sg-90664dea"] #security_groups= BadRequest => Must provide at least one security group. (Fog::AWS::EFS::Error) /usr/local/lib/ruby/gems/2.3.0/gems/excon-0.51.0/lib/excon/middlewares/expects.rb:6:in `response_call' /usr/local/lib/ruby/gems/2.3.0/gems/excon-0.51.0/lib/excon/middlewares/response_parser.rb:8:in `response_call' /usr/local/lib/ruby/gems/2.3.0/gems/excon-0.51.0/lib/excon/connection.rb:389:in `response' /usr/local/lib/ruby/gems/2.3.0/gems/excon-0.51.0/lib/excon/connection.rb:253:in `request' /usr/local/lib/ruby/gems/2.3.0/gems/fog-core-1.42.0/lib/fog/core/connection.rb:81:in `request' /usr/local/lib/ruby/gems/2.3.0/gems/fog-xml-0.1.2/lib/fog/xml/connection.rb:9:in `request' /Users/jlane/p/fog-aws/lib/fog/aws/efs.rb:134:in `_request' /Users/jlane/p/fog-aws/lib/fog/aws/efs.rb:129:in `request' /Users/jlane/p/fog-aws/lib/fog/aws/requests/efs/modify_mount_target_security_groups.rb:6:in `modify_mount_target_security_groups' /Users/jlane/p/fog-aws/lib/fog/aws/models/efs/mount_target.rb:41:in `security_groups=' /Users/jlane/p/fog-aws/tests/models/efs/mount_target_tests.rb:33:in `block (3 levels) in <top (required)>' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo.rb:79:in `instance_eval' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo.rb:79:in `tests' /Users/jlane/p/fog-aws/tests/models/efs/mount_target_tests.rb:32:in `block (2 levels) in <top (required)>' tests/helpers/model_helper.rb:12:in `block in model_tests' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo.rb:79:in `instance_eval' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo.rb:79:in `tests' tests/helpers/model_helper.rb:2:in `model_tests' /Users/jlane/p/fog-aws/tests/models/efs/mount_target_tests.rb:25:in `block in <top (required)>' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo.rb:79:in `instance_eval' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo.rb:79:in `tests' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo.rb:38:in `initialize' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo.rb:13:in `new' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo.rb:13:in `tests' /Users/jlane/p/fog-aws/tests/models/efs/mount_target_tests.rb:1:in `<top (required)>' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo/bin.rb:61:in `load' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo/bin.rb:61:in `block (2 levels) in run_in_thread' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo/bin.rb:58:in `each' /usr/local/lib/ruby/gems/2.3.0/gems/shindo-0.3.8/lib/shindo/bin.rb:58:in `block in run_in_thread'

when running live.

Ahh, fixed the request specs and broke the model specs >.<

Hold plz

ehowe · 2016-10-07T17:15:57Z

Ok, should work now.

lanej · 2016-10-07T17:44:44Z

tests/requests/efs/file_system_tests.rb

+
+    tests("#describe_mount_target_security_groups(#{mount_target_id})").formats(AWS::EFS::Formats::DESCRIBE_MOUNT_TARGET_SECURITY_GROUPS_FORMAT) do
+      result = Fog::AWS[:efs].describe_mount_target_security_groups(mount_target_id).body
+      returns([security_group.group_id]) { result["SecurityGroups"] }


got another error:

AWS::EFS | file systems (aws, efs) success #create_file_system('fogtesta35a') + returns "creating" + has proper format #describe_file_systems + has proper format #describe_file_systems(creation_token: fogtesta35a) + returns "fogtesta35a" + has proper format #describe_file_systems(id: fs-bd1cd7f4) + has proper format + invalid subnet ID: foobara35a + invalid file system ID: foobara35a + invalid security group ID: foobara35a #create_mount_target(fs-bd1cd7f4, subnet-0e617424) + has proper format #describe_mount_targets(file_system_id: fs-bd1cd7f4) + has proper format #describe_mount_target_security_groups(fsmt-03c7154a) + returns ["sg-2c2dfc56"] + has proper format + Must provide at least one security group. #modify_mount_target_security_groups(fsmt-03c7154a, [sg-a9577cd3]) + returns 204 #describe_mount_target_security_groups(fsmt-03c7154a) - returns ["sg-a9577cd3"] expected => ["sg-a9577cd3"] returned => ["sg-2c2dfc56"] Action? [c,q,r,?]? c

assertion appears correct in the context for the modify_mount_target_security_groups test above. maybe an eventually consistent thing ? could use a wait_for if that is the case.

It is definitely eventual consistency. I don't like the idea of a wait_for block here though, since there is nothing to definitively wait for. The state of the mount target doesn't change when you modify the security groups. Nothing can really be done except repeated queries, which seems hacky.

what about a wait_for that just waits for the value to be modified? would fit with the general expectation of the test.

Ok, I added that test.

* there seems to be a weird condition where sometimes the efs api returns one error, and other times a different error

* fix specs

ehowe · 2016-10-25T13:52:32Z

@lanej any chance we can get this merged in?

lanej · 2016-10-25T16:23:50Z

lib/fog/aws/requests/efs/describe_file_systems.rb

+                             raise Fog::AWS::EFS::NotFound.new("invalid file system ID: #{id}")
+                           end
+                         elsif creation_token = options[:creation_token]
+                           fs = self.data[:file_systems].values.detect { |file_system| file_system["CreationToken"] == creation_token }


- fs = self.data[:file_systems].values.detect { |file_system| file_system["CreationToken"] == creation_token } - [fs] + self.data[:file_systems].values.select { |file_system| file_system["CreationToken"] == creation_token }

lanej · 2016-10-25T16:24:29Z

thanks @ehowe

create, describe, and destroy elastic file systems

98f3af9

ehowe force-pushed the efs branch from 81c162c to 98f3af9 Compare October 5, 2016 18:35

lanej requested changes Oct 5, 2016

View reviewed changes

lanej approved these changes Oct 6, 2016

View reviewed changes

lanej requested changes Oct 6, 2016

View reviewed changes

lanej reviewed Oct 6, 2016

View reviewed changes

create, describe and destroy mount targets

295bac4

ehowe force-pushed the efs branch from 22ac00b to 295bac4 Compare October 7, 2016 02:49

Eugene Howe added 5 commits October 7, 2016 05:59

fix 1.8

8ff4737

describe mount target security groups

95284ad

modify mount target security groups

f620c87

remove debugging code

fd750c1

fix tests

4e2220d

lanej requested changes Oct 7, 2016

View reviewed changes

fix tests in real mode

fba07bb

lanej reviewed Oct 7, 2016

View reviewed changes

Eugene Howe added 5 commits October 10, 2016 08:46

fix some mocking behavior

824c271

fix describe mount targets

eb99011

NotFound changes

e2b1c3d

* there seems to be a weird condition where sometimes the efs api returns one error, and other times a different error

add wait_for to handle eventual consistency

fa33de7

* fix specs

accessing compute from efs does weird things

9393ae3

lanej reviewed Oct 25, 2016

View reviewed changes

lanej approved these changes Oct 25, 2016

View reviewed changes

lanej merged commit 67e0e28 into fog:master Oct 25, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create, describe, and destroy elastic file systems #304

create, describe, and destroy elastic file systems #304

ehowe commented Oct 5, 2016

lanej Oct 5, 2016

lanej Oct 5, 2016 •

edited

Loading

ehowe Oct 6, 2016

lanej Oct 6, 2016 •

edited

Loading

ehowe Oct 6, 2016

lanej Oct 6, 2016

lanej left a comment

lanej Oct 6, 2016

ehowe commented Oct 7, 2016

lanej left a comment

lanej Oct 7, 2016

ehowe Oct 7, 2016

ehowe commented Oct 7, 2016

lanej Oct 7, 2016

ehowe Oct 7, 2016

lanej Oct 10, 2016

ehowe Oct 12, 2016

ehowe commented Oct 25, 2016

lanej Oct 25, 2016

lanej commented Oct 25, 2016

create, describe, and destroy elastic file systems #304

create, describe, and destroy elastic file systems #304

Conversation

ehowe commented Oct 5, 2016

Choose a reason for hiding this comment

lanej Oct 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lanej Oct 6, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lanej left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehowe commented Oct 7, 2016

lanej left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehowe commented Oct 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ehowe commented Oct 25, 2016

Choose a reason for hiding this comment

lanej commented Oct 25, 2016

lanej Oct 5, 2016 •

edited

Loading

lanej Oct 6, 2016 •

edited

Loading