Separate Firewaller interface from Environ

Add type checks to code that actually uses that part of the interface. Signed-off-by: Michael McCracken <[email protected]>
ObviouslyGreen · Sep 25, 2017 · 1ce142c · 1ce142c
1 parent c13911b
commit 1ce142c
Show file tree

Hide file tree

Showing 11 changed files with 27 additions and 273 deletions.
diff --git a/environs/interface.go b/environs/interface.go
@@ -337,8 +337,6 @@ type Environ interface {
 	// when the Juju controller process is unavailable.
 	DestroyController(controllerUUID string) error
 
-	Firewaller
-
 	// Provider returns the EnvironProvider that created this Environ.
 	Provider() EnvironProvider
 

diff --git a/provider/common/firewaller.go b/provider/common/firewaller.go
diff --git a/provider/lxd/environ.go b/provider/lxd/environ.go
@@ -156,15 +156,6 @@ func (env *environ) Bootstrap(ctx environs.BootstrapContext, params environs.Boo
 // Destroy shuts down all known machines and destroys the rest of the
 // known environment.
 func (env *environ) Destroy() error {
-	rules, err := env.IngressRules()
-	if err != nil {
-		return errors.Trace(err)
-	}
-	if len(rules) > 0 {
-		if err := env.ClosePorts(rules); err != nil {
-			return errors.Trace(err)
-		}
-	}
 	if err := env.base.DestroyEnv(); err != nil {
 		return errors.Trace(err)
 	}

diff --git a/provider/lxd/environ_network.go b/provider/lxd/environ_network.go
diff --git a/provider/lxd/environ_network_test.go b/provider/lxd/environ_network_test.go
diff --git a/provider/lxd/environ_raw.go b/provider/lxd/environ_raw.go
@@ -11,7 +11,6 @@ import (
 
 	"github.com/juju/juju/environs"
 	"github.com/juju/juju/network"
-	"github.com/juju/juju/provider/common"
 	"github.com/juju/juju/tools/lxdclient"
 )
 
@@ -22,7 +21,6 @@ type rawProvider struct {
 	lxdProfiles
 	lxdImages
 	lxdStorage
-	common.Firewaller
 
 	remote lxdclient.Remote
 }
@@ -105,7 +103,6 @@ func newRawProviderFromConfig(config lxdclient.Config) (*rawProvider, error) {
 		lxdProfiles:  client,
 		lxdImages:    client,
 		lxdStorage:   client,
-		Firewaller:   common.NewFirewaller(),
 		remote:       config.Remote,
 	}, nil
 }

diff --git a/provider/lxd/environ_test.go b/provider/lxd/environ_test.go
@@ -16,7 +16,6 @@ import (
 	"github.com/juju/juju/cmd/modelcmd"
 	"github.com/juju/juju/environs"
 	envtesting "github.com/juju/juju/environs/testing"
-	"github.com/juju/juju/provider/common"
 	"github.com/juju/juju/provider/lxd"
 	coretesting "github.com/juju/juju/testing"
 	"github.com/juju/juju/tools/lxdclient"
@@ -123,9 +122,7 @@ func (s *environSuite) TestDestroy(c *gc.C) {
 	err := s.Env.Destroy()
 	c.Assert(err, jc.ErrorIsNil)
 
-	fwname := common.EnvFullName(s.Env.Config().UUID())
 	s.Stub.CheckCalls(c, []gitjujutesting.StubCall{
-		{"Ports", []interface{}{fwname}},
 		{"Destroy", nil},
 		{"StorageSupported", nil},
 		{"StoragePools", nil},
@@ -177,9 +174,7 @@ func (s *environSuite) TestDestroyController(c *gc.C) {
 	err := s.Env.DestroyController(s.Config.UUID())
 	c.Assert(err, jc.ErrorIsNil)
 
-	fwname := common.EnvFullName(s.Env.Config().UUID())
 	s.Stub.CheckCalls(c, []gitjujutesting.StubCall{
-		{"Ports", []interface{}{fwname}},
 		{"Destroy", nil},
 		{"StorageSupported", nil},
 		{"StoragePools", nil},

diff --git a/provider/lxd/testing_test.go b/provider/lxd/testing_test.go
@@ -312,10 +312,9 @@ func (s *BaseSuiteUnpatched) IsRunningLocally(c *gc.C) bool {
 type BaseSuite struct {
 	BaseSuiteUnpatched
 
-	Stub       *gitjujutesting.Stub
-	Client     *StubClient
-	Firewaller *stubFirewaller
-	Common     *stubCommon
+	Stub   *gitjujutesting.Stub
+	Client *StubClient
+	Common *stubCommon
 }
 
 func (s *BaseSuite) SetUpSuite(c *gc.C) {
@@ -339,7 +338,6 @@ func (s *BaseSuite) SetUpTest(c *gc.C) {
 			},
 		},
 	}
-	s.Firewaller = &stubFirewaller{stub: s.Stub}
 	s.Common = &stubCommon{stub: s.Stub}
 
 	// Patch out all expensive external deps.
@@ -350,7 +348,6 @@ func (s *BaseSuite) SetUpTest(c *gc.C) {
 		lxdProfiles:  s.Client,
 		lxdImages:    s.Client,
 		lxdStorage:   s.Client,
-		Firewaller:   s.Firewaller,
 		remote: lxdclient.Remote{
 			Cert: &lxdclient.Cert{
 				Name:    "juju",
@@ -661,38 +658,3 @@ func (conn *StubClient) VolumeUpdate(pool, volume string, update api.StorageVolu
 	conn.AddCall("VolumeUpdate", pool, volume, update)
 	return conn.NextErr()
 }
-
-// TODO(ericsnow) Move stubFirewaller to environs/testing or provider/common/testing.
-
-type stubFirewaller struct {
-	stub *gitjujutesting.Stub
-
-	PortRanges []network.IngressRule
-}
-
-func (fw *stubFirewaller) IngressRules(fwname string) ([]network.IngressRule, error) {
-	fw.stub.AddCall("Ports", fwname)
-	if err := fw.stub.NextErr(); err != nil {
-		return nil, errors.Trace(err)
-	}
-
-	return fw.PortRanges, nil
-}
-
-func (fw *stubFirewaller) OpenPorts(fwname string, rules ...network.IngressRule) error {
-	fw.stub.AddCall("OpenPorts", fwname, rules)
-	if err := fw.stub.NextErr(); err != nil {
-		return errors.Trace(err)
-	}
-
-	return nil
-}
-
-func (fw *stubFirewaller) ClosePorts(fwname string, rules ...network.IngressRule) error {
-	fw.stub.AddCall("ClosePorts", fwname, rules)
-	if err := fw.stub.NextErr(); err != nil {
-		return errors.Trace(err)
-	}
-
-	return nil
-}
diff --git a/provider/maas/environ.go b/provider/maas/environ.go
@@ -2026,22 +2026,6 @@ func (environ *maasEnviron) DestroyController(controllerUUID string) error {
 	return environ.Destroy()
 }
 
-// MAAS does not do firewalling so these port methods do nothing.
-func (*maasEnviron) OpenPorts([]network.IngressRule) error {
-	logger.Debugf("unimplemented OpenPorts() called")
-	return nil
-}
-
-func (*maasEnviron) ClosePorts([]network.IngressRule) error {
-	logger.Debugf("unimplemented ClosePorts() called")
-	return nil
-}
-
-func (*maasEnviron) IngressRules() ([]network.IngressRule, error) {
-	logger.Debugf("unimplemented Rules() called")
-	return nil, nil
-}
-
 func (*maasEnviron) Provider() environs.EnvironProvider {
 	return &providerInstance
 }