Merges 2.5 into 2.6 to bring forward PR juju#10288.

sdg0919 · Jun 6, 2019 · 359041f · 359041f
2 parents 6144682 + b00789c
commit 359041f
Show file tree

Hide file tree

Showing 5 changed files with 217 additions and 31 deletions.
diff --git a/provider/common/instance_configurator.go b/provider/common/instance_configurator.go
@@ -20,6 +20,7 @@ const (
 
 // Implementations of this interface should provide a way to configure external
 // IP allocation and add firewall functionality.
+//go:generate mockgen -package mocks -destination mocks/instance_configurator.go github.com/juju/juju/provider/common InstanceConfigurator
 type InstanceConfigurator interface {
 
 	// Close all ports.
@@ -116,9 +117,6 @@ func ConfigureExternalIpAddressCommands(apiPort int) []string {
 func (c *sshInstanceConfigurator) ConfigureExternalIpAddress(apiPort int) error {
 	cmds := ConfigureExternalIpAddressCommands(apiPort)
 	output, err := c.runCommand(strings.Join(cmds, "\n"))
-	if err != nil {
-		return errors.Errorf("failed to drop all ports: %s", output)
-	}
 	if err != nil {
 		return errors.Errorf("failed to allocate external IP address: %s", output)
 	}
@@ -139,7 +137,7 @@ func (c *sshInstanceConfigurator) ChangeIngressRules(ipAddress string, insert bo
 
 	output, err := c.runCommand(strings.Join(cmds, "\n"))
 	if err != nil {
-		return errors.Errorf("failed to configure ports on external network: %s", output)
+		return errors.Annotatef(err, "configuring ports for address %q: %s", ipAddress, output)
 	}
 	logger.Tracef("change ports output: %s", output)
 	return nil

diff --git a/provider/common/mocks/instance_configurator.go b/provider/common/mocks/instance_configurator.go
diff --git a/provider/oci/export_test.go b/provider/oci/export_test.go
@@ -3,18 +3,19 @@
 
 package oci
 
-import "github.com/juju/clock"
+import (
+	"github.com/juju/clock"
+	"github.com/juju/juju/provider/common"
+	"github.com/oracle/oci-go-sdk/core"
+)
 
 var (
 	InstanceTypes          = instanceTypes
 	RefreshImageCache      = refreshImageCache
-	FindInstanceSpec       = findInstanceSpec
-	GetImageType           = getImageType
 	ShapeSpecs             = shapeSpecs
 	SetImageCache          = setImageCache
 	NewInstance            = newInstance
 	MaxPollIterations      = &maxPollIterations
-	PollTime               = &pollTime
 	AllProtocols           = allProtocols
 	OciStorageProviderType = ociStorageProviderType
 	OciVolumeType          = ociVolumeType
@@ -25,3 +26,15 @@ var (
 func (e *Environ) SetClock(clock clock.Clock) {
 	e.clock = clock
 }
+
+func NewInstanceWithConfigurator(
+	raw core.Instance, env *Environ, factory func(string) common.InstanceConfigurator,
+) (*ociInstance, error) {
+	i, err := newInstance(raw, env)
+	if err != nil {
+		return nil, err
+	}
+
+	i.newInstanceConfigurator = factory
+	return i, nil
+}
diff --git a/provider/oci/instance.go b/provider/oci/instance.go
@@ -12,14 +12,14 @@ import (
 
 	"github.com/juju/errors"
 	"github.com/juju/juju/core/status"
+	ociCore "github.com/oracle/oci-go-sdk/core"
 
 	"github.com/juju/juju/core/instance"
 	envcontext "github.com/juju/juju/environs/context"
 	"github.com/juju/juju/environs/instances"
 	"github.com/juju/juju/network"
-	"github.com/juju/juju/provider/oci/common"
-
-	ociCore "github.com/oracle/oci-go-sdk/core"
+	"github.com/juju/juju/provider/common"
+	ocicommon "github.com/juju/juju/provider/oci/common"
 )
 
 const (
@@ -31,12 +31,15 @@ const (
 )
 
 type ociInstance struct {
-	arch     string
+	raw      ociCore.Instance
 	instType *instances.InstanceType
 	env      *Environ
-	mutex    sync.Mutex
+	arch     string
 	etag     *string
-	raw      ociCore.Instance
+
+	newInstanceConfigurator func(string) common.InstanceConfigurator
+
+	mutex sync.Mutex
 }
 
 type vnicWithIndex struct {
@@ -46,9 +49,9 @@ type vnicWithIndex struct {
 
 var _ instances.Instance = (*ociInstance)(nil)
 var maxPollIterations = 30
-var pollTime time.Duration = 10 * time.Second
+var pollTime = 10 * time.Second
 
-var statusMap map[ociCore.InstanceLifecycleStateEnum]status.Status = map[ociCore.InstanceLifecycleStateEnum]status.Status{
+var statusMap = map[ociCore.InstanceLifecycleStateEnum]status.Status{
 	ociCore.InstanceLifecycleStateProvisioning:  status.Provisioning,
 	ociCore.InstanceLifecycleStateRunning:       status.Running,
 	ociCore.InstanceLifecycleStateStarting:      status.Provisioning,
@@ -66,13 +69,13 @@ func newInstance(raw ociCore.Instance, env *Environ) (*ociInstance, error) {
 			"Instance response does not contain an ID",
 		)
 	}
-	instance := &ociInstance{
-		raw:  raw,
-		env:  env,
-		arch: "amd64",
-	}
 
-	return instance, nil
+	return &ociInstance{
+		raw:                     raw,
+		env:                     env,
+		arch:                    "amd64",
+		newInstanceConfigurator: common.NewSshInstanceConfigurator,
+	}, nil
 }
 
 // SetInstance sets the raw property of ociInstance{}
@@ -93,7 +96,7 @@ func (o *ociInstance) Id() instance.Id {
 // Status implements instances.Instance
 func (o *ociInstance) Status(ctx envcontext.ProviderCallContext) instance.Status {
 	if err := o.refresh(); err != nil {
-		common.HandleCredentialError(err, ctx)
+		ocicommon.HandleCredentialError(err, ctx)
 		return instance.Status{}
 	}
 	state, ok := statusMap[o.raw.LifecycleState]
@@ -135,8 +138,8 @@ func (o *ociInstance) getAddresses() ([]network.Address, error) {
 	if err != nil {
 		return nil, errors.Trace(err)
 	}
-	addresses := []network.Address{}
 
+	var addresses []network.Address
 	for _, val := range vnics {
 		if val.Vnic.PrivateIp != nil {
 			privateAddress := network.Address{
@@ -161,7 +164,7 @@ func (o *ociInstance) getAddresses() ([]network.Address, error) {
 // Addresses implements instances.Instance
 func (o *ociInstance) Addresses(ctx envcontext.ProviderCallContext) ([]network.Address, error) {
 	addresses, err := o.getAddresses()
-	common.HandleCredentialError(err, ctx)
+	ocicommon.HandleCredentialError(err, ctx)
 	return addresses, err
 }
 
@@ -180,7 +183,7 @@ func (o *ociInstance) waitForPublicIP(ctx envcontext.ProviderCallContext) error
 	for {
 		addresses, err := o.Addresses(ctx)
 		if err != nil {
-			common.HandleCredentialError(err, ctx)
+			ocicommon.HandleCredentialError(err, ctx)
 			return errors.Trace(err)
 		}
 		if iteration >= maxPollIterations {
@@ -217,7 +220,7 @@ func (o *ociInstance) deleteInstance(ctx envcontext.ProviderCallContext) error {
 	}
 	response, err := o.env.Compute.TerminateInstance(context.Background(), request)
 	if err != nil && !o.env.isNotFound(response.RawResponse) {
-		common.HandleCredentialError(err, ctx)
+		ocicommon.HandleCredentialError(err, ctx)
 		return err
 	}
 	iteration := 0
@@ -226,7 +229,7 @@ func (o *ociInstance) deleteInstance(ctx envcontext.ProviderCallContext) error {
 			if errors.IsNotFound(err) {
 				break
 			}
-			common.HandleCredentialError(err, ctx)
+			ocicommon.HandleCredentialError(err, ctx)
 			return err
 		}
 		logger.Infof("Waiting for machine to transition to Terminating: %s", o.raw.LifecycleState)
@@ -304,3 +307,62 @@ func (o *ociInstance) refresh() error {
 	o.raw = response.Instance
 	return nil
 }
+
+// OpenPorts (InstanceFirewaller) ensures that the input ingress rule is
+// permitted for machine with the input ID.
+func (o *ociInstance) OpenPorts(
+	ctx envcontext.ProviderCallContext, _ string, rules []network.IngressRule,
+) error {
+	client, err := o.getInstanceConfigurator(ctx)
+	if err != nil {
+		return errors.Trace(err)
+	}
+	return errors.Trace(client.ChangeIngressRules("", true, rules))
+}
+
+// OpenPorts (InstanceFirewaller) ensures that the input ingress rule is
+// restricted for machine with the input ID.
+func (o *ociInstance) ClosePorts(
+	ctx envcontext.ProviderCallContext, _ string, rules []network.IngressRule,
+) error {
+	client, err := o.getInstanceConfigurator(ctx)
+	if err != nil {
+		return errors.Trace(err)
+	}
+	return errors.Trace(client.ChangeIngressRules("", false, rules))
+}
+
+// IngressRules (InstanceFirewaller) returns the ingress rules that have been
+// applied to the input machine ID.
+func (o *ociInstance) IngressRules(
+	ctx envcontext.ProviderCallContext, _ string,
+) ([]network.IngressRule, error) {
+	client, err := o.getInstanceConfigurator(ctx)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	rules, err := client.FindIngressRules()
+	return rules, errors.Trace(err)
+}
+
+func (o *ociInstance) getInstanceConfigurator(
+	ctx envcontext.ProviderCallContext,
+) (common.InstanceConfigurator, error) {
+	addresses, err := o.Addresses(ctx)
+	if err != nil {
+		return nil, errors.Trace(err)
+	}
+
+	// Try to find a public address.
+	// Different models use different VCNs (and therefore subnets),
+	// so the cloud-local IPs are no good if a controller is trying to
+	// configure an instance in another model.
+	for _, addr := range addresses {
+		if addr.Scope == network.ScopePublic {
+			return o.newInstanceConfigurator(addr.Value), nil
+		}
+	}
+
+	return nil, errors.NotFoundf("public address for instance %q", o.Id())
+}
diff --git a/provider/oci/instance_test.go b/provider/oci/instance_test.go
@@ -7,14 +7,16 @@ import (
 	"context"
 	"strings"
 
-	gomock "github.com/golang/mock/gomock"
-	gc "gopkg.in/check.v1"
-
+	"github.com/golang/mock/gomock"
 	ociCore "github.com/oracle/oci-go-sdk/core"
+	gc "gopkg.in/check.v1"
 
 	"github.com/juju/juju/core/instance"
+	corenetwork "github.com/juju/juju/core/network"
 	"github.com/juju/juju/core/status"
 	"github.com/juju/juju/network"
+	"github.com/juju/juju/provider/common"
+	"github.com/juju/juju/provider/common/mocks"
 	"github.com/juju/juju/provider/oci"
 )
 
@@ -172,3 +174,31 @@ func (i *instanceSuite) TestAddressesNoPublicIP(c *gc.C) {
 	c.Check(addresses[0].Scope, gc.Equals, network.ScopeCloudLocal)
 	c.Check(addresses[1].Scope, gc.Equals, network.ScopePublic)
 }
+
+func (i *instanceSuite) TestInstanceConfiguratorUsesPublicAddress(c *gc.C) {
+	ctrl := i.patchEnv(c)
+	defer ctrl.Finish()
+
+	vnicID := "fakeVnicId"
+	i.setupListVnicsExpectations(i.testInstanceID, vnicID)
+
+	rules := []network.IngressRule{{
+		PortRange: corenetwork.PortRange{
+			FromPort: 1234,
+			ToPort:   1234,
+			Protocol: "tcp",
+		},
+	}}
+
+	ic := mocks.NewMockInstanceConfigurator(ctrl)
+	ic.EXPECT().ChangeIngressRules("", true, rules).Return(nil)
+
+	factory := func(addr string) common.InstanceConfigurator {
+		c.Assert(addr, gc.Equals, "2.2.2.2")
+		return ic
+	}
+
+	inst, err := oci.NewInstanceWithConfigurator(*i.ociInstance, i.env, factory)
+	c.Assert(err, gc.IsNil)
+	c.Assert(inst.OpenPorts(nil, "", rules), gc.IsNil)
+}