kfctl uses kfconfig (#4250)

* gcp use kfconfig * fix write config * fix * fix test * put back spec wip * fix test * fix test * fix * fix test * remove backfill app * kustomize changes * coordinator use kfconfig wip * coordinator can compile now * fix minikube * fix aws * fix test * fix existing * fix appdir * fix test * address comments * fix delete * fix test * fix? * populate err msg * remove app.yaml check * logs * hardcode UseIstio to true * set missing specs * remove dir check * unit test * fix unit test * fix converter for secret * rebase done. port changes from original pr * fix * fix delete
kubeflow · Oct 16, 2019 · 3c813ef · 3c813ef
1 parent 3b259dd
commit 3c813ef
Show file tree

Hide file tree

Showing 15 changed files with 297 additions and 283 deletions.
diff --git a/bootstrap/cmd/bootstrap/app/kfctlServer.go b/bootstrap/cmd/bootstrap/app/kfctlServer.go
@@ -103,7 +103,7 @@ func (s *kfctlServer) handleDeployment(r kfdefsv3.KfDef) (*kfdefsv3.KfDef, error
 		cfgFile := path.Join(r.Spec.AppDir, kftypes.KfConfigFile)
 		if _, err := os.Stat(cfgFile); os.IsNotExist(err) {
 			log.Infof("Creating cfgFile; %v", cfgFile)
-			newCfgFile, err := coordinator.CreateKfAppCfgFile(&r)
+			newCfgFile, err := coordinator.CreateKfAppCfgFileWithKfDef(&r)
 
 			if newCfgFile != cfgFile {
 				log.Errorf("Actual config file %v; doesn't match expected %v", newCfgFile, cfgFile)

diff --git a/bootstrap/cmd/kfctl/cmd/delete.go b/bootstrap/cmd/kfctl/cmd/delete.go
@@ -15,7 +15,6 @@
 package cmd
 
 import (
-	"errors"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -48,11 +47,12 @@ var deleteCmd = &cobra.Command{
 		if err != nil || kfApp == nil {
 			return fmt.Errorf("error loading kfapp: %v", err)
 		}
-		kfGetter, ok := kfApp.(coordinator.KfDefGetter)
-		if !ok {
-			return errors.New("internal error: coordinator does not implement KfDefGetter")
-		}
-		kfGetter.GetKfDef().Spec.DeleteStorage = deleteCfg.GetBool(string(kftypes.DELETE_STORAGE))
+		// TODO(lunkai): do we need set delete storage here?
+		// kfGetter, ok := kfApp.(coordinator.KfDefGetter)
+		// if !ok {
+		// 	return errors.New("internal error: coordinator does not implement KfDefGetter")
+		// }
+		// kfGetter.GetKfDef().Spec.DeleteStorage = deleteCfg.GetBool(string(kftypes.DELETE_STORAGE))
 		deleteErr := kfApp.Delete(kftypes.ALL)
 		if deleteErr != nil {
 			return fmt.Errorf("couldn't delete KfApp: %v", deleteErr)

diff --git a/bootstrap/go.mod b/bootstrap/go.mod
@@ -15,6 +15,7 @@ require (
 	github.com/gogo/protobuf v1.2.1
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
 	github.com/golang/protobuf v1.3.1
+	github.com/google/go-cmp v0.3.0
 	github.com/hashicorp/go-getter v1.0.2
 	github.com/imdario/mergo v0.3.7
 	github.com/kubeflow/kfctl/v3 v3.0.0-20190917231916-6ebaf60b014a

diff --git a/bootstrap/pkg/apis/apps/configconverters/converters.go b/bootstrap/pkg/apis/apps/configconverters/converters.go
@@ -136,6 +136,15 @@ func LoadConfigFromURI(configFile string) (*kfconfig.KfConfig, error) {
 	return converter.ToKfConfig(cwd, configFileBytes)
 }
 
+func isCwdEmpty() string {
+	cwd, _ := os.Getwd()
+	files, _ := ioutil.ReadDir(cwd)
+	if len(files) > 1 {
+		return ""
+	}
+	return cwd
+}
+
 func WriteConfigToFile(config kfconfig.KfConfig, filename string) error {
 	converters := map[string]Converter{
 		"v1alpha1": V1alpha1{},

diff --git a/bootstrap/pkg/apis/kferrors.go b/bootstrap/pkg/apis/kferrors.go
@@ -42,6 +42,11 @@ func (e *KfError) Error() string {
 		e.Code, e.Message)
 }
 
+func IsNotFound(e error) bool {
+	kfError, ok := e.(*KfError)
+	return ok && kfError.Code == int(NOT_FOUND)
+}
+
 // NewKfErrorWithMessage will propogate the error with the given message.
 //
 // TODO(jlewi): Not sure this is the best way to propogate the error messages and turn them

diff --git a/bootstrap/pkg/kfapp/aws/aws.go b/bootstrap/pkg/kfapp/aws/aws.go
@@ -39,6 +39,7 @@ import (
 	"github.com/ghodss/yaml"
 	kfapis "github.com/kubeflow/kubeflow/bootstrap/v3/pkg/apis"
 	kftypes "github.com/kubeflow/kubeflow/bootstrap/v3/pkg/apis/apps"
+	"github.com/kubeflow/kubeflow/bootstrap/v3/pkg/apis/apps/kfconfig"
 	kfdefs "github.com/kubeflow/kubeflow/bootstrap/v3/pkg/apis/apps/kfdef/v1alpha1"
 	"github.com/pkg/errors"
 	log "github.com/sirupsen/logrus"
@@ -69,7 +70,7 @@ const (
 // Aws implements KfApp Interface
 // It includes the KsApp along with additional Aws types
 type Aws struct {
-	kfDef     *kfdefs.KfDef
+	kfDef     *kfconfig.KfConfig
 	iamClient *iam.IAM
 	eksClient *eks.EKS
 	sess      *session.Session
@@ -79,7 +80,7 @@ type Aws struct {
 }
 
 // GetKfApp returns the aws kfapp. It's called by coordinator.GetKfApp
-func GetPlatform(kfdef *kfdefs.KfDef) (kftypes.Platform, error) {
+func GetPlatform(kfdef *kfconfig.KfConfig) (kftypes.Platform, error) {
 	session := session.Must(session.NewSession())
 
 	_aws := &Aws{
@@ -303,7 +304,7 @@ func (aws *Aws) updateClusterConfig(clusterConfigFile string) error {
 // ${KUBEFLOW_SRC}/${KFAPP}/aws_config -> destDir (dest)
 func (aws *Aws) generateInfraConfigs() error {
 	// 1. Copy and Paste all files from `sourceDir` to `destDir`
-	repo, ok := aws.kfDef.Status.ReposCache[kftypes.ManifestsRepoName]
+	repo, ok := aws.kfDef.GetRepoCache(kftypes.ManifestsRepoName)
 	if !ok {
 		err := fmt.Errorf("Repo %v not found in KfDef.Status.ReposCache", kftypes.ManifestsRepoName)
 		log.Errorf("%v", err)
@@ -476,13 +477,14 @@ func (aws *Aws) Init(resources kftypes.ResourceEnum) error {
 		}
 	}
 
-	createConfigErr := aws.kfDef.WriteToConfigFile()
-	if createConfigErr != nil {
-		return &kfapis.KfError{
-			Code:    int(kfapis.INVALID_ARGUMENT),
-			Message: fmt.Sprintf("Cannot create config file app.yaml in %v", aws.kfDef.Spec.AppDir),
-		}
-	}
+	// Should not need to write config here?
+	// createConfigErr := aws.kfDef.WriteToConfigFile()
+	// if createConfigErr != nil {
+	// 	return &kfapis.KfError{
+	// 		Code:    int(kfapis.INVALID_ARGUMENT),
+	// 		Message: fmt.Sprintf("Cannot create config file app.yaml in %v", aws.kfDef.Spec.AppDir),
+	// 	}
+	// }
 
 	return nil
 }
@@ -623,13 +625,14 @@ func (aws *Aws) Generate(resources kftypes.ResourceEnum) error {
 		}
 	}
 
-	if createConfigErr := aws.kfDef.WriteToConfigFile(); createConfigErr != nil {
-		return &kfapis.KfError{
-			Code: createConfigErr.(*kfapis.KfError).Code,
-			Message: fmt.Sprintf("Cannot create config file app.yaml in %v: %v", aws.kfDef.Spec.AppDir,
-				createConfigErr.(*kfapis.KfError).Message),
-		}
-	}
+	// Should not need to write config here.
+	// if createConfigErr := aws.kfDef.WriteToConfigFile(); createConfigErr != nil {
+	// 	return &kfapis.KfError{
+	// 		Code: createConfigErr.(*kfapis.KfError).Code,
+	// 		Message: fmt.Sprintf("Cannot create config file app.yaml in %v: %v", aws.kfDef.Spec.AppDir,
+	// 			createConfigErr.(*kfapis.KfError).Message),
+	// 	}
+	// }
 	return nil
 }