Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Joshd/sc 88868/reject requests for vm clusters with 1 nodes #326

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions cli/cmd/cluster_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cmd

import (
"fmt"
"strings"
"time"

"github.com/moby/moby/pkg/namesgenerator"
Expand All @@ -18,8 +17,17 @@ func (r *runners) InitClusterCreate(parent *cobra.Command) *cobra.Command {
cmd := &cobra.Command{
Use: "create",
Short: "Create test clusters",
Long: `Create test clusters`,
RunE: r.createCluster,
Long: `Create test clusters.

This is a beta feature, with some known limitations:
- K3s, Kind, kurl, helmvm and Openshift clusters only support a single node.
- EKS clusters may report ready before the nodes are completely online.
- kurl is the only supported distribution that can be upgraded.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line should be a limitation of the cluster upgrade command, not cluster create

Copy link
Member

@divolgin divolgin Sep 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's a fair warning here so it's not a surprise later. Maybe wording can be improved.

- Clusters cannot be resized. Create another cluster if you want to make changes, such as add another node.
- On cloud clusters, only one node group per cluster is supported.
- Multi-node support is available only for GKE and EKS.
- There is no support for IPv6.`,
RunE: r.createCluster,
}
parent.AddCommand(cmd)

Expand Down Expand Up @@ -78,11 +86,13 @@ func (r *runners) createAndWaitForCluster(opts kotsclient.CreateClusterOpts) (*t
return nil, errors.Wrap(err, "create cluster")
}

if ve != nil && len(ve.Errors) > 0 {
if len(ve.SupportedDistributions) > 0 {
_ = print.ClusterVersions("table", r.w, ve.SupportedDistributions)
if ve != nil && ve.Message != "" {
if ve.ValidationError != nil && len(ve.ValidationError.Errors) > 0 {
if len(ve.ValidationError.SupportedDistributions) > 0 {
_ = print.ClusterVersions("table", r.w, ve.ValidationError.SupportedDistributions)
}
}
return nil, fmt.Errorf("%s", errors.New(strings.Join(ve.Errors, ",")))
return nil, fmt.Errorf("%s", ve.Message)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errors.New(ve.Message)

}

if opts.DryRun {
Expand Down
4 changes: 2 additions & 2 deletions cli/cmd/cluster_ls.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ import (
func (r *runners) InitClusterList(parent *cobra.Command) *cobra.Command {
cmd := &cobra.Command{
Use: "ls",
Short: "list test clusters",
Long: `list test clusters`,
Short: "List test clusters",
Long: `List test clusters`,
RunE: r.listClusters,
}
parent.AddCommand(cmd)
Expand Down
4 changes: 2 additions & 2 deletions cli/cmd/cluster_rm.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@ import (
func (r *runners) InitClusterRemove(parent *cobra.Command) *cobra.Command {
cmd := &cobra.Command{
Use: "rm ID [ID …]",
Short: "remove test clusters",
Short: "Remove test clusters",
Long: `Removes a cluster immediately.

You can specify the --all flag to terminate all clusters.`,
RunE: r.removeCluster,
RunE: r.removeCluster,
}
parent.AddCommand(cmd)

Expand Down
8 changes: 4 additions & 4 deletions cli/cmd/cluster_versions.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ import (

func (r *runners) InitClusterVersions(parent *cobra.Command) *cobra.Command {
cmd := &cobra.Command{
Use: "versions",
Short: "list cluster versions",
Long: `list cluster versions`,
RunE: r.listClusterVersions,
Use: "versions",
Short: "List cluster versions",
Long: `List cluster versions`,
RunE: r.listClusterVersions,
}
parent.AddCommand(cmd)

Expand Down
8 changes: 6 additions & 2 deletions cli/print/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@ var clustersTmplRowSrc = `{{ range . -}}
var clustersTmplSrc = fmt.Sprintln(clustersTmplHeaderSrc) + clustersTmplRowSrc

var clusterVersionsTmplSrc = `Supported Kubernetes distributions and versions are:
DISTRIBUTION VERSION INSTANCE_TYPES
{{ range $d := . -}}{{ $d.Name }} {{ range $i, $v := $d.Versions -}}{{if $i}}, {{end}}{{ $v }}{{ end }} {{ range $i, $it := $d.InstanceTypes -}}{{if $i}}, {{end}}{{ $it }}{{ end }}
{{ range $d := . -}}
DISTRIBUTION: {{ $d.Name }}
• VERSIONS: {{ range $i, $v := $d.Versions -}}{{if $i}}, {{end}}{{ $v }}{{ end }}
• INSTANCE TYPES: {{ range $i, $it := $d.InstanceTypes -}}{{if $i}}, {{end}}{{ $it }}{{ end }}
• MAX NODES: {{ $d.NodesMax }}

{{ end }}`

var clustersTmpl = template.Must(template.New("clusters").Funcs(funcs).Parse(clustersTmplSrc))
Expand Down
43 changes: 31 additions & 12 deletions pkg/kotsclient/cluster_create.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package kotsclient

import (
"encoding/json"
"fmt"
"net/http"
"strings"

"github.com/replicatedhq/replicated/pkg/platformclient"
"github.com/replicatedhq/replicated/pkg/types"
)

Expand Down Expand Up @@ -34,12 +36,25 @@ type CreateClusterOpts struct {
DryRun bool
}

type CreateClusterErrorResponse struct {
Error CreateClusterErrorError `json:"Error"`
}

type CreateClusterErrorError struct {
Message string `json:"message"`
MaxDiskGiB int64 `json:"maxDiskGiB,omitempty"`
MaxEKS int64 `json:"maxEKS,omitempty"`
MaxGKE int64 `json:"maxGKE,omitempty"`
MaxAKS int64 `json:"maxAKS,omitempty"`
ValidationError *ClusterValidationError `json:"validationError,omitempty"`
}

type ClusterValidationError struct {
Errors []string `json:"errors"`
SupportedDistributions []*types.ClusterVersion `json:"supported_distributions"`
}

func (c *VendorV3Client) CreateCluster(opts CreateClusterOpts) (*types.Cluster, *ClusterValidationError, error) {
func (c *VendorV3Client) CreateCluster(opts CreateClusterOpts) (*types.Cluster, *CreateClusterErrorError, error) {
req := CreateClusterRequest{
Name: opts.Name,
KubernetesDistribution: opts.KubernetesDistribution,
Expand All @@ -57,31 +72,35 @@ func (c *VendorV3Client) CreateCluster(opts CreateClusterOpts) (*types.Cluster,
return c.doCreateClusterRequest(req)
}

func (c *VendorV3Client) doCreateClusterRequest(req CreateClusterRequest) (*types.Cluster, *ClusterValidationError, error) {
func (c *VendorV3Client) doCreateClusterRequest(req CreateClusterRequest) (*types.Cluster, *CreateClusterErrorError, error) {
resp := CreateClusterResponse{}
endpoint := "/v3/cluster"
err := c.DoJSON("POST", endpoint, http.StatusCreated, req, &resp)
if err != nil {
if strings.Contains(err.Error(), " 400: ") {
// to avoid a lot of brittle string parsing, we make the request again with
// a dry-run flag and expect to get the same result back
ve, _ := c.doCreateClusterDryRunRequest(req)
if ve != nil && len(ve.Errors) > 0 {
return nil, ve, nil
// if err is APIError and the status code is 400, then we have a validation error
// and we can return the validation error
if apiErr, ok := err.(platformclient.APIError); ok {
if apiErr.StatusCode == http.StatusBadRequest {
veResp := &CreateClusterErrorResponse{}
if jsonErr := json.Unmarshal(apiErr.Body, veResp); jsonErr != nil {
return nil, nil, fmt.Errorf("unmarshal validation error response: %w", err)
}
return nil, &veResp.Error, nil
}
}

return nil, nil, err
}

return resp.Cluster, nil, nil
}

func (c *VendorV3Client) doCreateClusterDryRunRequest(req CreateClusterRequest) (*ClusterValidationError, error) {
resp := ClusterValidationError{}
func (c *VendorV3Client) doCreateClusterDryRunRequest(req CreateClusterRequest) (*CreateClusterErrorError, error) {
resp := CreateClusterErrorResponse{}
endpoint := "/v3/cluster?dry-run=true"
err := c.DoJSON("POST", endpoint, http.StatusOK, req, &resp)
if err != nil {
return nil, err
}
return &resp, nil
return &resp.Error, nil
}
18 changes: 11 additions & 7 deletions pkg/kotsclient/cluster_upgrade.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package kotsclient

import (
"encoding/json"
"fmt"
"net/http"
"strings"

"github.com/replicatedhq/replicated/pkg/platformclient"
"github.com/replicatedhq/replicated/pkg/types"
)

Expand Down Expand Up @@ -40,12 +41,15 @@ func (c *VendorV3Client) doUpgradeClusterRequest(clusterID string, req UpgradeCl
endpoint := fmt.Sprintf("/v3/cluster/%s/upgrade", clusterID)
err := c.DoJSON("POST", endpoint, http.StatusOK, req, &resp)
if err != nil {
if strings.Contains(err.Error(), " 400: ") {
// to avoid a lot of brittle string parsing, we make the request again with
// a dry-run flag and expect to get the same result back
ve, _ := c.doUpgradeClusterDryRunRequest(clusterID, req)
if ve != nil && len(ve.Errors) > 0 {
return nil, ve, nil
// if err is APIError and the status code is 400, then we have a validation error
// and we can return the validation error
if apiErr, ok := err.(platformclient.APIError); ok {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

errors.Cause(err).(platformclient.APIError)

if apiErr.StatusCode == http.StatusBadRequest {
veResp := &ClusterValidationError{}
if jsonErr := json.Unmarshal(apiErr.Body, veResp); jsonErr != nil {
return nil, nil, fmt.Errorf("unmarshal validation error response: %w", err)
}
return nil, veResp, nil
}
}
return nil, nil, err
Expand Down
1 change: 1 addition & 0 deletions pkg/types/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,5 @@ type ClusterVersion struct {
Name string `json:"short_name"`
Versions []string `json:"versions"`
InstanceTypes []string `json:"instance_types"`
NodesMax int `json:"nodes_max"`
}