Skip to content

Commit

Permalink
Merge pull request #2216 from zerbitx/ENG-4851
Browse files Browse the repository at this point in the history
feat: prompt for missing vcluster name in create command
  • Loading branch information
FabianKramm authored Oct 14, 2024
2 parents aabc672 + 6e637f1 commit e73dfd2
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 25 deletions.
28 changes: 15 additions & 13 deletions .github/workflows/e2e.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ jobs:
- id: set-paths-matrix
run: |
set -x
sudo apt-get install -y jq
sudo apt-get install -y jq yq
paths=$(ls -d ./test/e2e*)
echo "matrix=$(printf '%s\n' "${paths}" | jq -R . | jq -cs .)" >> "$GITHUB_OUTPUT"
outputs:
Expand Down Expand Up @@ -139,13 +139,13 @@ jobs:
name: e2e-binaries
path: ./test/*/*.test
retention-days: 7

vcluster-install-delete:
name: Install and delete virtual cluster
needs:
- build-and-push-syncer-image
- build-vcluster-cli

runs-on: ubuntu-latest
steps:
- name: Checkout repository
Expand Down Expand Up @@ -179,26 +179,26 @@ jobs:
uses: actions/download-artifact@v4
with:
name: vcluster_syncer

- name: Setup environment
run: |
kind load image-archive vcluster_syncer
chmod +x vcluster && sudo mv vcluster /usr/bin
sudo apt-get install -y sed
- name: Run tests - install and delete virtual cluster using kubectl
run: |
set -x
./hack/vcluster-install-scripts/test-kubectl-install.sh
- name: Run tests - install and delete virtual cluster using helm
run: |
set -x
./hack/vcluster-install-scripts/test-helm-install.sh
download-latest-cli:
Expand All @@ -214,7 +214,7 @@ jobs:
name: vcluster-current
path: ./vcluster-current
retention-days: 7

upgrade-test:
name: test if we can upgrade from older version
needs:
Expand Down Expand Up @@ -273,12 +273,13 @@ jobs:
chmod +x ./vcluster-current
kind load image-archive vcluster_syncer
yq eval '.controlPlane.distro.${{ matrix.distribution }}.enabled = true' > ./test/vcluster-current.yaml
./vcluster-current create ${{ env.VCLUSTER_SUFFIX }} -n ${{ env.VCLUSTER_NAMESPACE }} \
--create-namespace \
--debug \
--connect=false \
--distro=${{ matrix.distribution }}
-f ./test/vcluster-current.yaml
./hack/wait-for-pod.sh -l app=${{ env.VCLUSTER_SUFFIX }} -n ${{ env.VCLUSTER_NAMESPACE }}
Expand All @@ -289,8 +290,9 @@ jobs:
sed -i "s|REPLACE_REPOSITORY_NAME|${{ env.REPOSITORY_NAME }}|g" test/commonValues.yaml
sed -i "s|REPLACE_TAG_NAME|${{ env.TAG_NAME }}|g" test/commonValues.yaml
yq eval -i '.controlPlane.distro.${{ matrix.distribution }}.enabled = true' test/commonValues.yaml
./vcluster-dev/vcluster create vcluster --distro=${{ matrix.distribution }} \
./vcluster-dev/vcluster create vcluster \
--connect=false \
--upgrade \
--local-chart-dir ./chart \
Expand Down Expand Up @@ -411,6 +413,7 @@ jobs:
sed -i "s|REPLACE_REPOSITORY_NAME|${{ env.REPOSITORY_NAME }}|g" ${{ matrix.test-suite-path }}/../commonValues.yaml
sed -i "s|REPLACE_TAG_NAME|${{ env.TAG_NAME }}|g" ${{ matrix.test-suite-path }}/../commonValues.yaml
yq eval -i '.controlPlane.distro.${{ matrix.distribution }}.enabled = true' ${{ matrix.test-suite-path }}/../commonValues.yaml
kind load image-archive vcluster_syncer
Expand All @@ -420,7 +423,6 @@ jobs:
--create-namespace \
--debug \
--connect=false \
--distro=${{ matrix.distribution }} \
--local-chart-dir ./chart \
-f ./test/commonValues.yaml \
$haValues \
Expand Down
2 changes: 1 addition & 1 deletion Justfile
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ e2e distribution="k3s" path="./test/e2e" multinamespace="false": create-kind &&

sed -i.bak "s|REPLACE_REPOSITORY_NAME|vcluster|g" dist/commonValues.yaml
sed -i.bak "s|REPLACE_TAG_NAME|e2e-latest|g" dist/commonValues.yaml
yq eval -i '.controlPlane.distro.{{distribution}}.enabled = true' dist/commonValues.yaml
rm dist/commonValues.yaml.bak

sed -i.bak "s|kind-control-plane|vcluster-control-plane|g" dist/commonValues.yaml
Expand All @@ -123,7 +124,6 @@ e2e distribution="k3s" path="./test/e2e" multinamespace="false": create-kind &&
--create-namespace \
--debug \
--connect=false \
--distro={{ distribution }} \
--local-chart-dir ./chart/ \
-f ./dist/commonValues.yaml \
-f {{ path }}/values.yaml \
Expand Down
11 changes: 9 additions & 2 deletions cmd/vclusterctl/cmd/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmd
import (
"cmp"
"context"
"errors"
"fmt"

"github.com/loft-sh/log"
Expand Down Expand Up @@ -44,12 +45,18 @@ Example:
vcluster create test --namespace test
#######################################################
`,
Args: util.VClusterNameOnlyValidator,
RunE: func(cobraCmd *cobra.Command, args []string) error {
newArgs, err := util.PromptForArgs(cmd.log, args, "vcluster name")
if err != nil && errors.Is(err, util.ErrNonInteractive) {
if err := util.VClusterNameOnlyValidator(cobraCmd, args); err != nil {
return err
}
}

// Check for newer version
upgrade.PrintNewerVersionWarning()

return cmd.Run(cobraCmd.Context(), args)
return cmd.Run(cobraCmd.Context(), newArgs)
},
}

Expand Down
21 changes: 14 additions & 7 deletions cmd/vclusterctl/cmd/platform/add/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,26 @@ package add
import (
"cmp"
"context"
"errors"
"fmt"
"os"
"os/exec"
"time"

"github.com/loft-sh/log"
"github.com/sirupsen/logrus"
kerrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/util/wait"

managementv1 "github.com/loft-sh/api/v4/pkg/apis/management/v1"
storagev1 "github.com/loft-sh/api/v4/pkg/apis/storage/v1"
"github.com/loft-sh/log"
"github.com/loft-sh/vcluster/pkg/cli/flags"
"github.com/loft-sh/vcluster/pkg/cli/util"
"github.com/loft-sh/vcluster/pkg/platform"
"github.com/loft-sh/vcluster/pkg/platform/clihelper"
"github.com/loft-sh/vcluster/pkg/platform/kube"
"github.com/loft-sh/vcluster/pkg/upgrade"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
kerrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/tools/clientcmd"
)
Expand Down Expand Up @@ -60,12 +61,18 @@ Example:
vcluster platform add cluster my-cluster
########################################################
`,
Args: cobra.ExactArgs(1),
RunE: func(cobraCmd *cobra.Command, args []string) error {
newArgs, err := util.PromptForArgs(cmd.Log, args, "cluster name")
if err != nil && errors.Is(err, util.ErrNonInteractive) {
if err := cobra.ExactArgs(1)(cobraCmd, args); err != nil {
return err
}
}

// Check for newer version
upgrade.PrintNewerVersionWarning()

return cmd.Run(cobraCmd.Context(), args)
return cmd.Run(cobraCmd.Context(), newArgs)
},
}

Expand Down
11 changes: 9 additions & 2 deletions cmd/vclusterctl/cmd/platform/create/vcluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package create

import (
"context"
"errors"

"github.com/loft-sh/log"
"github.com/loft-sh/vcluster/pkg/cli"
Expand Down Expand Up @@ -39,12 +40,18 @@ Example:
vcluster platform create vcluster test --namespace test
#########################################################################
`,
Args: util.VClusterNameOnlyValidator,
RunE: func(cobraCmd *cobra.Command, args []string) error {
newArgs, err := util.PromptForArgs(cmd.log, args, "vcluster name")
if err != nil && errors.Is(err, util.ErrNonInteractive) {
if err := util.VClusterNameOnlyValidator(cobraCmd, args); err != nil {
return err
}
}

// Check for newer version
upgrade.PrintNewerVersionWarning()

return cmd.Run(cobraCmd.Context(), args)
return cmd.Run(cobraCmd.Context(), newArgs)
},
}

Expand Down
32 changes: 32 additions & 0 deletions pkg/cli/util/args.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
package util

import (
"errors"
"fmt"
"strings"

"github.com/loft-sh/log"
"github.com/loft-sh/log/survey"
"github.com/loft-sh/log/terminal"
"github.com/spf13/cobra"
)

Expand All @@ -14,6 +18,8 @@ var (
VClusterNameOnlyUseLine string

VClusterNameOnlyValidator cobra.PositionalArgs

ErrNonInteractive = errors.New("terminal is not interactive")
)

func init() {
Expand Down Expand Up @@ -67,3 +73,29 @@ func NamedPositionalArgsValidator(failMissing, failExtra bool, expectedArgs ...s
return nil
}
}

// PromptForArgs expects that the terminal is interactive and the number of args, matched the number of argNames, in the
// order they should appear and will prompt one by one for the missing args adding them to the args slice and returning
// a new set for a command to use. It returns the args, rather than a nil slice so they're unaltered in error cases.
func PromptForArgs(l log.Logger, args []string, argNames ...string) ([]string, error) {
if !terminal.IsTerminalIn {
return args, ErrNonInteractive
}

if len(args) == len(argNames) {
return args, nil
}

for i := range argNames[len(args):] {
answer, err := l.Question(&survey.QuestionOptions{
Question: fmt.Sprintf("Please specify %s", argNames[i]),
})

if err != nil {
return args, err
}
args = append(args, answer)
}

return args, nil
}

0 comments on commit e73dfd2

Please sign in to comment.