ControllerServiceType should be omitable, fix tests;

hatched · Apr 8, 2019 · a7eb4be · a7eb4be
1 parent 61645e0
commit a7eb4be
Show file tree

Hide file tree

Showing 12 changed files with 69 additions and 41 deletions.
diff --git a/.gitignore b/.gitignore
@@ -22,4 +22,4 @@ vendor/
 *.pyc
 .vscode/
 juju-backup-*.tar.gz
-.editorconfig
+.editorconfig
diff --git a/api/state.go b/api/state.go
@@ -17,7 +17,6 @@ import (
 	"gopkg.in/juju/names.v2"
 	"gopkg.in/macaroon-bakery.v2-unstable/httpbakery"
 	"gopkg.in/macaroon.v2-unstable"
-	goyaml "gopkg.in/yaml.v2"
 
 	"github.com/juju/juju/api/base"
 	"github.com/juju/juju/api/charmrevisionupdater"
@@ -61,10 +60,6 @@ func (st *state) Login(tag names.Tag, password, nonce string, macaroons []macaro
 		)
 	}
 	err := st.APICall("Admin", 3, "", "Login", request, &result)
-
-	data, err := goyaml.Marshal(result)
-	logger.Criticalf("Login request -> %+v, result -> \n%s", request, string(data))
-
 	if err != nil {
 		var resp params.RedirectInfoResult
 		if params.IsRedirect(err) {

diff --git a/caas/kubernetes/provider/bootstrap.go b/caas/kubernetes/provider/bootstrap.go
@@ -4,14 +4,14 @@
 package provider
 
 import (
-	"context"
 	"fmt"
 	"path/filepath"
 	"reflect"
 	"runtime"
 	"strings"
 	"time"
 
+	jujuclock "github.com/juju/clock"
 	"github.com/juju/errors"
 	"github.com/juju/loggo"
 	"gopkg.in/juju/names.v2"
@@ -44,7 +44,8 @@ var (
 )
 
 type controllerStack struct {
-	ctx environs.BootstrapContext
+	ctx   environs.BootstrapContext
+	clock jujuclock.Clock
 
 	stackName   string
 	stackLabels map[string]string
@@ -103,19 +104,19 @@ func DecideControllerNamespace(controllerName string) string {
 	return "controller-" + controllerName
 }
 
-// ReScheduler executes a function and keep retrying if the function returns an expected error.
+// ReScheduler executes a function and keep retrying until timeout if the function returns an expected error.
 func ReScheduler(
 	do func() error,
 	expectedErrorChecker func(error) bool,
+	clock jujuclock.Clock,
 	timeout time.Duration,
 ) (err error) {
-	ctx, cancel := context.WithTimeout(context.Background(), timeout)
-	defer cancel()
 	retryInterval := 3 * time.Second
 	for {
 		select {
-		case <-time.After(retryInterval):
+		case <-clock.After(retryInterval):
 			err = do()
+			logger.Criticalf("do --> %v", err)
 			if err == nil {
 				return nil
 			}
@@ -127,9 +128,8 @@ func ReScheduler(
 				retryInterval,
 			)
 			// got expected error, keep retrying.
-		case <-ctx.Done():
-			err = ctx.Err()
-			return errors.Trace(err)
+		case <-clock.After(timeout):
+			return errors.Timeoutf("%q exceeded", timeout.String())
 		}
 	}
 }
@@ -175,6 +175,7 @@ func newcontrollerStack(
 
 	cs := controllerStack{
 		ctx:         ctx,
+		clock:       broker.clock,
 		stackName:   stackName,
 		stackLabels: map[string]string{labelApplication: stackName},
 		broker:      broker,
@@ -379,9 +380,9 @@ func (c controllerStack) createControllerService() error {
 			"setting controller public DNS to %v", publicAddress,
 		)
 	}
-
+	// keep polling the svc public address until timeout in 3 minutes(this should be enough time to provision the LB).
 	return errors.Trace(
-		ReScheduler(publicAddressPoller, errors.IsNotProvisioned, 2*time.Minute),
+		ReScheduler(publicAddressPoller, errors.IsNotProvisioned, c.clock, 3*time.Minute),
 	)
 }
 

diff --git a/caas/kubernetes/provider/bootstrap_test.go b/caas/kubernetes/provider/bootstrap_test.go
@@ -4,6 +4,8 @@
 package provider_test
 
 import (
+	"time"
+
 	"github.com/golang/mock/gomock"
 	jc "github.com/juju/testing/checkers"
 	gc "gopkg.in/check.v1"
@@ -54,7 +56,8 @@ func (s *bootstrapSuite) SetUpTest(c *gc.C) {
 	s.controllerUUID = "9bec388c-d264-4cde-8b29-3e675959157a"
 
 	s.controllerCfg = testing.FakeControllerConfig()
-	pcfg, err := podcfg.NewBootstrapControllerPodConfig(s.controllerCfg, controllerName, "bionic")
+	s.controllerCfg[controller.APIPort] = 17070
+	pcfg, err := podcfg.NewBootstrapControllerPodConfig(s.controllerCfg, controllerName, "bionic", "ClusterIP")
 	c.Assert(err, jc.ErrorIsNil)
 
 	pcfg.JujuVersion = jujuversion.Current
@@ -85,6 +88,7 @@ func (s *bootstrapSuite) SetUpTest(c *gc.C) {
 		StatePort:    123,
 		APIPort:      456,
 	}
+	pcfg.Bootstrap.ControllerConfig = s.controllerCfg
 	s.pcfg = pcfg
 	s.controllerStackerGetter = func() provider.ControllerStackerForTest {
 		controllerStacker, err := provider.NewcontrollerStackForTest(
@@ -192,17 +196,31 @@ func (s *bootstrapSuite) TestBootstrap(c *gc.C) {
 			Type:     core.ServiceType("ClusterIP"),
 			Ports: []core.ServicePort{
 				{
-					Name:       "mongodb",
-					TargetPort: intstr.FromInt(37017),
-					Port:       37017,
-					Protocol:   "TCP",
+					Name:       "api-server",
+					TargetPort: intstr.FromInt(17070),
+					Port:       17070,
 				},
+			},
+		},
+	}
+
+	svcProvisioned := core.Service{
+		ObjectMeta: v1.ObjectMeta{
+			Name:      "juju-controller-test-service",
+			Labels:    map[string]string{"juju-app": "juju-controller-test"},
+			Namespace: s.getNamespace(),
+		},
+		Spec: core.ServiceSpec{
+			Selector: map[string]string{"juju-app": "juju-controller-test"},
+			Type:     core.ServiceType("ClusterIP"),
+			Ports: []core.ServicePort{
 				{
 					Name:       "api-server",
 					TargetPort: intstr.FromInt(17070),
 					Port:       17070,
 				},
 			},
+			ClusterIP: "1.1.1.1",
 		},
 	}
 
@@ -526,6 +544,16 @@ test -e /var/lib/juju/agents/machine-0/agent.conf || ./jujud bootstrap-state /va
 		s.mockServices.EXPECT().Create(svc).Times(1).
 			Return(svc, nil),
 
+		// below calls are for GetService.
+		s.mockServices.EXPECT().List(v1.ListOptions{LabelSelector: "juju-app==juju-controller-test", IncludeUninitialized: true}).Times(1).
+			Return(&core.ServiceList{Items: []core.Service{svcProvisioned}}, nil),
+		s.mockStatefulSets.EXPECT().Get("juju-operator-juju-controller-test", v1.GetOptions{IncludeUninitialized: true}).Times(1).
+			Return(nil, s.k8sNotFoundError()),
+		s.mockStatefulSets.EXPECT().Get("juju-controller-test", v1.GetOptions{IncludeUninitialized: false}).Times(1).
+			Return(nil, s.k8sNotFoundError()),
+		s.mockDeployments.EXPECT().Get("juju-controller-test", v1.GetOptions{IncludeUninitialized: false}).Times(1).
+			Return(nil, s.k8sNotFoundError()),
+
 		// ensure shared-secret secret.
 		s.mockSecrets.EXPECT().Get("juju-controller-test-secret", v1.GetOptions{IncludeUninitialized: true}).AnyTimes().
 			Return(nil, s.k8sNotFoundError()),
@@ -570,5 +598,20 @@ test -e /var/lib/juju/agents/machine-0/agent.conf || ./jujud bootstrap-state /va
 		s.mockStatefulSets.EXPECT().Create(statefulSetSpec).Times(1).
 			Return(statefulSetSpec, nil),
 	)
-	c.Assert(controllerStacker.Deploy(), jc.ErrorIsNil)
+
+	errChan := make(chan error)
+	go func() {
+		errChan <- controllerStacker.Deploy()
+	}()
+
+	err = s.clock.WaitAdvance(3*time.Second, testing.LongWait, 2)
+	c.Assert(err, jc.ErrorIsNil)
+
+	select {
+	case err := <-errChan:
+		c.Assert(err, jc.ErrorIsNil)
+	case <-time.After(coretesting.LongWait):
+		c.Fatalf("timed out waiting for deploy error")
+	}
+
 }
diff --git a/caas/kubernetes/provider/k8s.go b/caas/kubernetes/provider/k8s.go
@@ -1678,11 +1678,6 @@ func (k *kubernetesClient) ensureK8sService(spec *core.Service) error {
 	return errors.Trace(err)
 }
 
-// getK8sService gets a k8s service resource by service name.
-func (k *kubernetesClient) getK8sService(name string) error {
-	return nil
-}
-
 // deleteService deletes a service resource.
 func (k *kubernetesClient) deleteService(deploymentName string) error {
 	services := k.CoreV1().Services(k.namespace)

diff --git a/caas/kubernetes/provider/k8s_test.go b/caas/kubernetes/provider/k8s_test.go
@@ -647,7 +647,8 @@ func (s *K8sBrokerSuite) assertDestroy(c *gc.C, isController bool, destroyFunc f
 	go func(w *watch.RaceFreeFakeWatcher, clk *testclock.Clock) {
 		for _, f := range []func(runtime.Object){w.Add, w.Modify, w.Delete} {
 			if !w.IsStopped() {
-				clk.WaitAdvance(time.Second, testing.LongWait, 1)
+				err := clk.WaitAdvance(time.Second, testing.LongWait, 1)
+				c.Assert(err, jc.ErrorIsNil)
 				f(ns)
 			}
 		}

diff --git a/caas/kubernetes/provider/services.go b/caas/kubernetes/provider/services.go
@@ -7,9 +7,6 @@ import (
 	core "k8s.io/api/core/v1"
 )
 
-// // ControllerServiceTypeKey defines controller service type key.
-// const ControllerServiceTypeKey = "controller-service-type"
-
 var preferredControllerServiceTypes = map[string]core.ServiceType{
 	K8sCloudAzure:    core.ServiceTypeLoadBalancer,
 	K8sCloudCDK:      core.ServiceTypeLoadBalancer,

diff --git a/cloudconfig/podcfg/podcfg_test.go b/cloudconfig/podcfg/podcfg_test.go
@@ -60,6 +60,7 @@ func (*podcfgSuite) TestOperatorImagesDefaultRepo(c *gc.C) {
 		cfg,
 		"controller-1",
 		"kubernetes",
+		"LoadBalancer",
 	)
 	c.Assert(err, jc.ErrorIsNil)
 	podConfig.JujuVersion = version.MustParse("6.6.6")
@@ -74,6 +75,7 @@ func (*podcfgSuite) TestOperatorImagesCustomRepo(c *gc.C) {
 		cfg,
 		"controller-1",
 		"kubernetes",
+		"LoadBalancer",
 	)
 	c.Assert(err, jc.ErrorIsNil)
 	podConfig.JujuVersion = version.MustParse("6.6.6")

diff --git a/cmd/juju/commands/bootstrap.go b/cmd/juju/commands/bootstrap.go
@@ -12,6 +12,7 @@ import (
 	"strings"
 	"time"
 
+	jujuclock "github.com/juju/clock"
 	"github.com/juju/cmd"
 	"github.com/juju/errors"
 	"github.com/juju/gnuflag"
@@ -738,7 +739,6 @@ See `[1:] + "`juju kill-controller`" + `.`)
 	bootstrapParams.CloudCredential = credentials.credential
 	bootstrapParams.CloudCredentialName = credentials.name
 
-	logger.Criticalf("bootstrapParams --> %+v", bootstrapParams.ControllerServiceType)
 	bootstrapFuncs := getBootstrapFuncs()
 	if err = bootstrapFuncs.Bootstrap(
 		modelcmd.BootstrapContext(ctx),
@@ -796,7 +796,7 @@ See `[1:] + "`juju kill-controller`" + `.`)
 		)
 	}
 
-	if err = k8sprovider.ReScheduler(controllerDataRefresher, errors.IsNotProvisioned, 2*time.Minute); err != nil {
+	if err = k8sprovider.ReScheduler(controllerDataRefresher, errors.IsNotProvisioned, jujuclock.WallClock, 3*time.Minute); err != nil {
 		return errors.Trace(err)
 	}
 

diff --git a/environs/bootstrap.go b/environs/bootstrap.go
@@ -61,10 +61,6 @@ type BootstrapParams struct {
 	// that rely on it for selecting images. This will be empty for
 	// providers that do not implements simplestreams.HasRegion.
 	ImageMetadata []*imagemetadata.ImageMetadata
-
-	// // ControllerServiceType is the service type to use for a k8s controller.
-	// // Optional: only for k8s controller.
-	// ControllerServiceType string
 }
 
 // CloudBootstrapFinalizer is a function returned from Environ.Bootstrap.

diff --git a/environs/bootstrap/bootstrap.go b/environs/bootstrap/bootstrap.go
@@ -202,7 +202,6 @@ func bootstrapCAAS(
 	args BootstrapParams,
 	bootstrapParams environs.BootstrapParams,
 ) error {
-	logger.Criticalf("args -> %+v", args)
 	if args.BuildAgent {
 		return errors.NewNotSupported(nil, "--build-agent when bootstrapping a k8s controller")
 	}

diff --git a/environs/bootstrap/config.go b/environs/bootstrap/config.go
@@ -133,7 +133,6 @@ func (c Config) Validate() error {
 // $JUJU_DATA/ca-private-key.pem. If none of these are set, an
 // error is returned.
 func NewConfig(attrs map[string]interface{}) (Config, error) {
-	logger.Criticalf("attrs 1-> %+v", attrs)
 	coerced, err := configChecker.Coerce(attrs, nil)
 	if err != nil {
 		return Config{}, errors.Trace(err)
@@ -147,7 +146,6 @@ func NewConfig(attrs map[string]interface{}) (Config, error) {
 	if svcType, ok := attrs[ControllerServiceTypeKey].(string); ok {
 		config.ControllerServiceType = svcType
 	}
-	logger.Criticalf("attrs 2-> %+v", attrs)
 	if adminSecret, ok := attrs[AdminSecretKey].(string); ok {
 		config.AdminSecret = adminSecret
 	} else {
@@ -243,6 +241,7 @@ var configChecker = schema.FieldMap(schema.Fields{
 	CACertKey + "-path":        schema.Omit,
 	CAPrivateKeyKey:            schema.Omit,
 	CAPrivateKeyKey + "-path":  schema.Omit,
+	ControllerServiceTypeKey:   schema.Omit,
 	BootstrapTimeoutKey:        DefaultBootstrapSSHTimeout,
 	BootstrapRetryDelayKey:     DefaultBootstrapSSHRetryDelay,
 	BootstrapAddressesDelayKey: DefaultBootstrapSSHAddressesDelay,