Merge "Allow to setup timeout for get kubeconfig request"

pkg/api/v1alpha1/cluster_map_types.go

@@ -65,6 +65,7 @@ type KubeconfigSourceFilesystem struct {
 // KubeconfigSourceClusterAPI get kubeconfig from clusterAPI parent cluster
 type KubeconfigSourceClusterAPI struct {
 	NamespacedName `json:"clusterNamespacedName,omitempty"`
+	Timeout        string `json:"timeout,omitempty"`
 }
 
 // KubeconfigSourceBundle get kubeconfig from bundle

pkg/cluster/command.go

@@ -18,8 +18,6 @@ import (
 	"fmt"
 	"io"
 
-	airshipv1 "opendev.org/airship/airshipctl/pkg/api/v1alpha1"
-	"opendev.org/airship/airshipctl/pkg/clusterctl/client"
 	"opendev.org/airship/airshipctl/pkg/config"
 	"opendev.org/airship/airshipctl/pkg/k8s/kubeconfig"
 	"opendev.org/airship/airshipctl/pkg/log"
@@ -78,11 +76,6 @@ func (cmd *GetKubeconfigCommand) RunE(cfgFactory config.Factory, writer io.Write
 		return err
 	}
 
-	client, err := client.NewClient(helper.TargetPath(), log.DebugEnabled(), airshipv1.DefaultClusterctl())
-	if err != nil {
-		return err
-	}
-
 	var siteWide bool
 	if cmd.ClusterName == "" {
 		siteWide = true
@@ -90,7 +83,6 @@ func (cmd *GetKubeconfigCommand) RunE(cfgFactory config.Factory, writer io.Write
 
 	kubeconf := kubeconfig.NewBuilder().
 		WithBundle(helper.PhaseConfigBundle()).
-		WithClusterctlClient(client).
 		WithClusterMap(cMap).
 		WithClusterName(cmd.ClusterName).
 		WithTempRoot(helper.WorkDir()).

pkg/clusterctl/client/client.go

@@ -44,7 +44,6 @@ const (
 type Interface interface {
 	Init(kubeconfigPath, kubeconfigContext string) error
 	Move(fromKubeconfigPath, fromKubeconfigContext, toKubeconfigPath, toKubeconfigContext, namespace string) error
-	GetKubeconfig(options *GetKubeconfigOptions) (string, error)
 	Render(options RenderOptions) ([]byte, error)
 }
 
@@ -65,11 +64,8 @@ type RenderOptions struct {
 
 // GetKubeconfigOptions carries all the options to retrieve kubeconfig from parent cluster
 type GetKubeconfigOptions struct {
-	// Path to parent kubeconfig file
+	// Timeout is the maximum length of time to retrieve kubeconfig
-	ParentKubeconfigPath string
+	Timeout string
-	// Specify context within the kubeconfig file. If empty, cluster client
-	// will use the current context.
-	ParentKubeconfigContext string
 	// Namespace is the namespace in which secret is placed.
 	ManagedClusterNamespace string
 	// ManagedClusterName is the name of the managed cluster.
@@ -168,15 +164,3 @@ func (c *Client) Render(renderOptions RenderOptions) ([]byte, error) {
 	}
 	return components.Yaml()
 }
-
-// GetKubeconfig is a wrapper for related cluster-api function
-func (c *Client) GetKubeconfig(options *GetKubeconfigOptions) (string, error) {
-	return c.clusterctlClient.GetKubeconfig(clusterctlclient.GetKubeconfigOptions{
-		Kubeconfig: clusterctlclient.Kubeconfig{
-			Path:    options.ParentKubeconfigPath,
-			Context: options.ParentKubeconfigContext,
-		},
-		Namespace:           options.ManagedClusterNamespace,
-		WorkloadClusterName: options.ManagedClusterName,
-	})
-}

pkg/k8s/kubeconfig/builder.go

@@ -17,6 +17,7 @@ package kubeconfig
 import (
 	"fmt"
 
+	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/tools/clientcmd/api"
 
@@ -25,12 +26,10 @@ import (
 	"opendev.org/airship/airshipctl/pkg/clusterctl/client"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/fs"
+	"opendev.org/airship/airshipctl/pkg/k8s/utils"
 	"opendev.org/airship/airshipctl/pkg/log"
 )
 
-// KubeconfigDefaultFileName is a default name for kubeconfig
-const KubeconfigDefaultFileName = "kubeconfig"
-
 // NewBuilder returns instance of kubeconfig builder.
 func NewBuilder() *Builder {
 	return &Builder{
@@ -46,8 +45,8 @@ type Builder struct {
 	root        string
 
 	bundle       document.Bundle
+	client       corev1.CoreV1Interface
 	clusterMap   clustermap.ClusterMap
-	clusterctlClient client.Interface
 	fs           fs.FileSystem
 	siteKubeconf *api.Config
 }
@@ -76,19 +75,18 @@ func (b *Builder) WithTempRoot(root string) *Builder {
 	return b
 }
 
-// WithClusterctlClient this is used if u want to inject your own clusterctl
-// mostly needed for tests
-func (b *Builder) WithClusterctlClient(c client.Interface) *Builder {
-	b.clusterctlClient = c
-	return b
-}
-
 // WithFilesystem allows to set filesystem
 func (b *Builder) WithFilesystem(fs fs.FileSystem) *Builder {
 	b.fs = fs
 	return b
 }
 
+// WithCoreV1Client allows to set core v1 client, use for unit tests only
+func (b *Builder) WithCoreV1Client(c corev1.CoreV1Interface) *Builder {
+	b.client = c
+	return b
+}
+
 // SiteWide allows to build kubeconfig for the entire site.
 // If set to true ClusterName will be ignored, since all clusters are requested.
 func (b *Builder) SiteWide(t bool) *Builder {
@@ -229,18 +227,18 @@ func (b *Builder) fromClusterAPI(clusterName string, ref v1alpha1.KubeconfigSour
 		}
 		defer cleanup()
 
-		if b.clusterctlClient == nil {
+		if b.client == nil {
-			b.clusterctlClient, err = client.NewClient("", log.DebugEnabled(), v1alpha1.DefaultClusterctl())
+			clientSet, err := utils.FactoryFromKubeConfig(f, parentContext, utils.SetTimeout("30s")).KubernetesClientSet()
 			if err != nil {
 				return nil, err
 			}
+			b.client = clientSet.CoreV1()
 		}
 
 		log.Debugf("Getting child kubeconfig from parent, parent context '%s', parent kubeconfig '%s'",
 			parentContext, f)
-		return FromSecret(b.clusterctlClient, &client.GetKubeconfigOptions{
+		return FromSecret(b.client, &client.GetKubeconfigOptions{
-			ParentKubeconfigPath:    f,
+			Timeout:                 ref.Timeout,
-			ParentKubeconfigContext: parentContext,
 			ManagedClusterNamespace: ref.Namespace,
 			ManagedClusterName:      ref.Name,
 		})()
@@ -257,7 +255,7 @@ func (b *Builder) alreadyBuilt(clusterContext string) (bool, *api.Config) {
 	// resulting and existing context names must be the same, otherwise error will be returned
 	clusterKubeconfig, err := extractContext(clusterContext, clusterContext, kubeconfBytes)
 	if err != nil {
-		log.Debugf("Received error when extacting context, ignoring kubeconfig. Error: %v", err)
+		log.Debugf("Received error when extracting context, ignoring kubeconfig. Error: %v", err)
 		return false, nil
 	}
 

pkg/k8s/kubeconfig/builder_test.go

@@ -22,15 +22,16 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
+	apiv1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/tools/clientcmd"
 
 	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
 	"opendev.org/airship/airshipctl/pkg/cluster/clustermap"
-	"opendev.org/airship/airshipctl/pkg/clusterctl/client"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/fs"
 	"opendev.org/airship/airshipctl/pkg/k8s/kubeconfig"
-	"opendev.org/airship/airshipctl/testutil/clusterctl"
 	testfs "opendev.org/airship/airshipctl/testutil/fs"
 )
 
@@ -99,7 +100,7 @@ func TestBuilderClusterctl(t *testing.T) {
 
 		expectedContexts, expectedClusters, expectedAuthInfos []string
 		clusterMap                                            clustermap.ClusterMap
-		clusterctlClient                                      client.Interface
+		client                                                corev1.CoreV1Interface
 		fs                                                    fs.FileSystem
 	}{
 		{
@@ -219,23 +220,18 @@ func TestBuilderClusterctl(t *testing.T) {
 					}, nil
 				},
 			},
-			clusterctlClient: func() client.Interface {
+			client: func() MockCoreV1Interface {
-				c := &clusterctl.MockInterface{
+				ms := &SecretMockInterface{
 					Mock: mock.Mock{},
 				}
-				c.On("GetKubeconfig", &client.GetKubeconfigOptions{
+				ms.On("Get", parentClusterID+"-kubeconfig", metav1.GetOptions{}).
-					ParentKubeconfigPath:    kubeconfigPath,
+					Once().Return(&apiv1.Secret{Data: map[string][]byte{"value": []byte(testKubeconfigString)}}, nil)
-					ParentKubeconfigContext: parentClusterID,
+				ms.On("Get", childClusterID+"-kubeconfig", metav1.GetOptions{}).
-					ManagedClusterNamespace: clustermap.DefaultClusterAPIObjNamespace,
+					Once().Return(&apiv1.Secret{Data: map[string][]byte{"value": []byte(testKubeconfigStringSecond)}}, nil)
-					ManagedClusterName:      childClusterID,
+				mc := MockCoreV1Interface{MockSecrets: func(s string) corev1.SecretInterface {
-				}).Once().Return(testKubeconfigString, nil)
+					return ms
-				c.On("GetKubeconfig", &client.GetKubeconfigOptions{
+				}}
-					ParentKubeconfigPath:    kubeconfigPath,
+				return mc
-					ParentKubeconfigContext: parentParentClusterID,
-					ManagedClusterNamespace: clustermap.DefaultClusterAPIObjNamespace,
-					ManagedClusterName:      parentClusterID,
-				}).Once().Return(testKubeconfigStringSecond, nil)
-				return c
 			}(),
 		},
 		{
@@ -267,7 +263,7 @@ func TestBuilderClusterctl(t *testing.T) {
 				WithClusterName(tt.requestedClusterName).
 				WithBundle(testBundle).
 				WithTempRoot(tt.tempRoot).
-				WithClusterctlClient(tt.clusterctlClient).
+				WithCoreV1Client(tt.client).
 				WithFilesystem(tt.fs).
 				SiteWide(tt.siteWide).
 				Build()

pkg/k8s/kubeconfig/errors.go

@@ -48,3 +48,20 @@ type ErrUnknownKubeconfigSourceType struct {
 func (e *ErrUnknownKubeconfigSourceType) Error() string {
 	return fmt.Sprintf("unknown source type %s", e.Type)
 }
+
+// ErrClusterNameEmpty returned when cluster name is not provided
+type ErrClusterNameEmpty struct {
+}
+
+func (e ErrClusterNameEmpty) Error() string {
+	return "cluster name is not defined"
+}
+
+// ErrMalformedKubeconfig error returned if kubeconfig is empty
+type ErrMalformedKubeconfig struct {
+	ClusterName string
+}
+
+func (e ErrMalformedKubeconfig) Error() string {
+	return fmt.Sprintf("retrieved kubeconfig for cluster '%s' is empty", e.ClusterName)
+}

pkg/k8s/kubeconfig/kubeconfig.go

@@ -15,9 +15,13 @@
 package kubeconfig
 
 import (
+	"fmt"
 	"io"
-	"log"
+	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/wait"
+	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	"k8s.io/client-go/tools/clientcmd"
 	"k8s.io/client-go/tools/clientcmd/api"
 	"sigs.k8s.io/yaml"
@@ -26,12 +30,14 @@ import (
 	"opendev.org/airship/airshipctl/pkg/clusterctl/client"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/fs"
+	"opendev.org/airship/airshipctl/pkg/log"
 	"opendev.org/airship/airshipctl/pkg/util"
 )
 
 const (
-	// KubeconfigPrefix is a prefix that is added when writing temporary kubeconfig files
+	// Prefix is a prefix that is added when writing temporary kubeconfig files
-	KubeconfigPrefix = "kubeconfig-"
+	Prefix         = "kubeconfig-"
+	defaultTimeout = 30 * time.Second
 )
 
 // Interface provides a uniform way to interact with kubeconfig file
@@ -100,13 +106,39 @@ func FromAPIalphaV1(apiObj *v1alpha1.KubeConfig) KubeSourceFunc {
 }
 
 // FromSecret returns KubeSource type, uses client interface to kubernetes cluster
-func FromSecret(c client.Interface, o *client.GetKubeconfigOptions) KubeSourceFunc {
+func FromSecret(c corev1.CoreV1Interface, o *client.GetKubeconfigOptions) KubeSourceFunc {
 	return func() ([]byte, error) {
-		data, err := c.GetKubeconfig(o)
+		if o.ManagedClusterName == "" {
+			return nil, ErrClusterNameEmpty{}
+		}
+		if o.ManagedClusterNamespace == "" {
+			o.ManagedClusterNamespace = "default"
+		}
+
+		data, exist, secretName := new([]byte), new(bool), fmt.Sprintf("%s-kubeconfig", o.ManagedClusterName)
+		fn := func() (bool, error) {
+			secret, err := c.Secrets(o.ManagedClusterNamespace).Get(secretName, metav1.GetOptions{})
 			if err != nil {
+				log.Printf("get kubeconfig from secret failed, retrying, reason: %v", err)
+				return false, nil
+			}
+
+			if *data, *exist = secret.Data["value"]; *exist && len(*data) > 0 {
+				return true, nil
+			}
+			return true, ErrMalformedKubeconfig{ClusterName: o.ManagedClusterName}
+		}
+
+		duration, err := time.ParseDuration(o.Timeout)
+		if err != nil || duration == 0 {
+			duration = defaultTimeout
+		}
+
+		if err = wait.PollImmediate(time.Second, duration, fn); err != nil {
 			return nil, err
 		}
-		return []byte(data), nil
+
+		return *data, nil
 	}
 }
 
@@ -197,7 +229,7 @@ func (k *kubeConfig) WriteTempFile(root string) (string, Cleanup, error) {
 	if err != nil {
 		return "", nil, err
 	}
-	file, err := k.fileSystem.TempFile(root, KubeconfigPrefix)
+	file, err := k.fileSystem.TempFile(root, Prefix)
 	if err != nil {
 		log.Printf("Failed to write temporary file, error %v", err)
 		return "", nil, err

pkg/k8s/kubeconfig/kubeconfig_test.go

@@ -23,7 +23,14 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
+	apiv1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/wait"
+	"k8s.io/apimachinery/pkg/watch"
+	corev1 "k8s.io/client-go/kubernetes/typed/core/v1"
 	v1 "k8s.io/client-go/tools/clientcmd/api/v1"
 	kustfs "sigs.k8s.io/kustomize/api/filesys"
 
@@ -112,46 +119,121 @@ func TestKubeconfigContent(t *testing.T) {
 	assert.Equal(t, expectedData, actualData)
 }
 
-type MockClientInterface struct {
+type MockCoreV1Interface struct {
-	MockGetKubeconfig func(options *client.GetKubeconfigOptions) (string, error)
+	MockSecrets func(string) corev1.SecretInterface
-	client.Interface
+	corev1.CoreV1Interface
 }
 
-func (c MockClientInterface) GetKubeconfig(o *client.GetKubeconfigOptions) (string, error) {
+func (c MockCoreV1Interface) Secrets(n string) corev1.SecretInterface {
-	return c.MockGetKubeconfig(o)
+	return c.MockSecrets(n)
+}
+
+var _ corev1.SecretInterface = &SecretMockInterface{}
+
+type SecretMockInterface struct {
+	mock.Mock
+}
+
+func (s *SecretMockInterface) Create(_ *apiv1.Secret) (*apiv1.Secret, error) {
+	panic("implement me")
+}
+
+func (s *SecretMockInterface) Update(_ *apiv1.Secret) (*apiv1.Secret, error) {
+	panic("implement me")
+}
+
+func (s *SecretMockInterface) Delete(_ string, _ *metav1.DeleteOptions) error {
+	panic("implement me")
+}
+
+func (s *SecretMockInterface) DeleteCollection(_ *metav1.DeleteOptions, _ metav1.ListOptions) error {
+	panic("implement me")
+}
+
+func (s *SecretMockInterface) Get(name string, options metav1.GetOptions) (*apiv1.Secret, error) {
+	args := s.Called(name, options)
+	expectedResult, ok := args.Get(0).(*apiv1.Secret)
+	if !ok {
+		return nil, fmt.Errorf("wrong input")
+	}
+	return expectedResult, args.Error(1)
+}
+
+func (s *SecretMockInterface) List(_ metav1.ListOptions) (*apiv1.SecretList, error) {
+	panic("implement me")
+}
+
+func (s *SecretMockInterface) Watch(_ metav1.ListOptions) (watch.Interface, error) {
+	panic("implement me")
+}
+
+func (s *SecretMockInterface) Patch(_ string, _ types.PatchType, _ []byte, _ ...string) (*apiv1.Secret, error) {
+	panic("implement me")
 }
 
 func TestFromSecret(t *testing.T) {
 	tests := []struct {
 		name         string
-		expectedData string
+		options      *client.GetKubeconfigOptions
-		err          error
+		getSecret    *apiv1.Secret
+		getErr       error
+		expectedData []byte
+		expectedErr  error
 	}{
 		{
-			name:         "valid kubeconfig",
+			name:        "empty cluster name",
-			expectedData: testValidKubeconfig,
+			options:     &client.GetKubeconfigOptions{},
-			err:          nil,
+			expectedErr: kubeconfig.ErrClusterNameEmpty{},
 		},
 		{
-			name:         "failed to get kubeconfig",
+			name:        "multiple retries and error",
-			expectedData: "",
+			options:     &client.GetKubeconfigOptions{ManagedClusterName: "cluster", Timeout: "1s"},
-			err:          errors.New("error"),
+			getErr:      errors.New("error"),
+			expectedErr: wait.ErrWaitTimeout,
+		},
+		{
+			name:        "empty secret object",
+			options:     &client.GetKubeconfigOptions{ManagedClusterName: "cluster"},
+			getSecret:   &apiv1.Secret{},
+			expectedErr: kubeconfig.ErrMalformedKubeconfig{ClusterName: "cluster"},
+		},
+		{
+			name:        "empty data value",
+			options:     &client.GetKubeconfigOptions{ManagedClusterName: "cluster"},
+			getSecret:   &apiv1.Secret{Data: map[string][]byte{"value": {}}},
+			expectedErr: kubeconfig.ErrMalformedKubeconfig{ClusterName: "cluster"},
+		},
+		{
+			name:         "successfully get kubeconfig",
+			options:      &client.GetKubeconfigOptions{ManagedClusterName: "cluster"},
+			getSecret:    &apiv1.Secret{Data: map[string][]byte{"value": []byte(testValidKubeconfig)}},
+			expectedData: []byte(testValidKubeconfig),
 		},
 	}
 
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
-			cl := MockClientInterface{
+			sm := &SecretMockInterface{
-				MockGetKubeconfig: func(_ *client.GetKubeconfigOptions) (string, error) { return tt.expectedData, tt.err },
+				Mock: mock.Mock{},
 			}
-			kubeconf, err := kubeconfig.FromSecret(cl, nil)()
+			sm.On("Get", tt.options.ManagedClusterName+"-kubeconfig", metav1.GetOptions{}).
-			if tt.err != nil {
+				Return(tt.getSecret, tt.getErr)
+
+			coreV1Interface := MockCoreV1Interface{
+				MockSecrets: func(s string) corev1.SecretInterface {
+					return sm
+				},
+			}
+
+			kubeconf, err := kubeconfig.FromSecret(coreV1Interface, tt.options)()
+			if tt.expectedErr != nil {
 				require.Error(t, err)
-				assert.Nil(t, kubeconf)
+				require.Equal(t, tt.expectedErr, err)
+				require.Nil(t, kubeconf)
 			} else {
 				require.NoError(t, err)
-				assert.Equal(t, []byte(tt.expectedData), kubeconf)
+				require.Equal(t, tt.expectedData, kubeconf)
 			}
 		})
 	}
@@ -423,10 +505,10 @@ type fakeReaderWriter struct {
 var _ io.Reader = fakeReaderWriter{}
 var _ io.Writer = fakeReaderWriter{}
 
-func (f fakeReaderWriter) Read(p []byte) (n int, err error) {
+func (f fakeReaderWriter) Read(_ []byte) (n int, err error) {
 	return 0, f.readErr
 }
 
-func (f fakeReaderWriter) Write(p []byte) (n int, err error) {
+func (f fakeReaderWriter) Write(_ []byte) (n int, err error) {
 	return 0, f.writeErr
 }

pkg/phase/client.go

@@ -24,7 +24,6 @@ import (
 	"sigs.k8s.io/yaml"
 
 	"opendev.org/airship/airshipctl/pkg/api/v1alpha1"
-	cctlclient "opendev.org/airship/airshipctl/pkg/clusterctl/client"
 	"opendev.org/airship/airshipctl/pkg/container"
 	"opendev.org/airship/airshipctl/pkg/document"
 	"opendev.org/airship/airshipctl/pkg/events"
@@ -102,19 +101,10 @@ func (p *phase) executor(docFactory document.DocFactoryFunc,
 		return nil, err
 	}
 
-	cctlClient, err := cctlclient.NewClient(
-		p.helper.PhaseBundleRoot(),
-		log.DebugEnabled(),
-		v1alpha1.DefaultClusterctl())
-	if err != nil {
-		return nil, err
-	}
-
 	kubeconf := kubeconfig.NewBuilder().
 		WithBundle(p.helper.PhaseConfigBundle()).
 		WithClusterMap(cMap).
 		WithTempRoot(p.helper.WorkDir()).
-		WithClusterctlClient(cctlClient).
 		WithClusterName(p.apiObj.ClusterName).
 		SiteWide(p.apiObj.Config.SiteWideKubeconfig).
 		Build()
@@ -311,7 +301,7 @@ func (p *plan) Run(ro ifc.RunOptions) error {
 }
 
 // Status returns the status of phases in a given plan
-func (p *plan) Status(options ifc.StatusOptions) (ifc.PlanStatus, error) {
+func (p *plan) Status(_ ifc.StatusOptions) (ifc.PlanStatus, error) {
 	for _, step := range p.apiObj.Phases {
 		phase, err := p.phaseClient.PhaseByID(ifc.ID{Name: step.Name})
 		if err != nil {