From 9a76bd6744eb11a7c306018ce5665b7dcfd36114 Mon Sep 17 00:00:00 2001 From: Daniel Isen Date: Mon, 25 Nov 2024 17:23:07 -0500 Subject: [PATCH] [CLOUDGA-24851] Remove root node info from cluster creation --- Makefile | 3 + cmd/cluster/create_cluster.go | 36 +----- internal/client/client.go | 216 ++++++++++++---------------------- 3 files changed, 87 insertions(+), 168 deletions(-) diff --git a/Makefile b/Makefile index 5739f74f..ce33d545 100644 --- a/Makefile +++ b/Makefile @@ -38,3 +38,6 @@ update-cli: clean: rm -rf ybm + +fmt: + go fmt ./... diff --git a/cmd/cluster/create_cluster.go b/cmd/cluster/create_cluster.go index f97e1d49..9c2930e7 100644 --- a/cmd/cluster/create_cluster.go +++ b/cmd/cluster/create_cluster.go @@ -18,9 +18,9 @@ package cluster import ( "fmt" "os" - "strconv" "strings" + "encoding/base64" "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/spf13/viper" @@ -29,7 +29,6 @@ import ( ybmAuthClient "github.com/yugabyte/ybm-cli/internal/client" "github.com/yugabyte/ybm-cli/internal/formatter" ybmclient "github.com/yugabyte/yugabytedb-managed-go-client-internal" - "encoding/base64" ) func encodeBase64(s string) string { @@ -55,25 +54,6 @@ var createClusterCmd = &cobra.Command{ password := credentials["password"] regionInfoMapList := []map[string]string{} changedRegionInfo := cmd.Flags().Changed("region-info") - changedNodeInfo := cmd.Flags().Changed("node-config") - - defaultNumCores := 0 - defaultDiskSizeGb := 0 - defaultDiskIops := 0 - if changedNodeInfo { - nodeConfig, _ := cmd.Flags().GetStringToInt("node-config") - numCores, ok := nodeConfig["num-cores"] - - if ok { - defaultNumCores = numCores - } - if diskSizeGb, ok := nodeConfig["disk-size-gb"]; ok { - defaultDiskSizeGb = diskSizeGb - } - if diskIops, ok := nodeConfig["disk-iops"]; ok { - defaultDiskIops = diskIops - } - } if changedRegionInfo { regionInfoList, _ := cmd.Flags().GetStringArray("region-info") @@ -120,14 +100,11 @@ var createClusterCmd = &cobra.Command{ if _, ok := regionInfoMap["num-nodes"]; !ok { logrus.Fatalln("Number of nodes not specified in region info") } - if _, ok := regionInfoMap["num-cores"]; !ok && defaultNumCores > 0 { - regionInfoMap["num-cores"] = strconv.Itoa(defaultNumCores) - } - if _, ok := regionInfoMap["disk-size-gb"]; !ok && defaultDiskSizeGb > 0 { - regionInfoMap["disk-size-gb"] = strconv.Itoa(defaultDiskSizeGb) + if _, ok := regionInfoMap["num-cores"]; !ok { + logrus.Fatalln("Number of cores not specified in region info") } - if _, ok := regionInfoMap["disk-iops"]; !ok && defaultDiskIops > 0 { - regionInfoMap["disk-iops"] = strconv.Itoa(defaultDiskIops) + if _, ok := regionInfoMap["disk-size-gb"]; !ok { + logrus.Fatalln("Disk size not specified in region info") } regionInfoMapList = append(regionInfoMapList, regionInfoMap) @@ -231,9 +208,8 @@ func init() { If specified, all parameters for that provider are mandatory.`) createClusterCmd.Flags().String("fault-tolerance", "", "[OPTIONAL] Fault tolerance of the cluster. The possible values are NONE, NODE, ZONE, or REGION. Default NONE.") createClusterCmd.Flags().Int32("num-faults-to-tolerate", 0, "[OPTIONAL] The number of domain faults to tolerate for the level specified. The possible values are 0 for NONE, 1 for ZONE and [1-3] for anything else. Defaults to 0 for NONE, 1 otherwise.") - createClusterCmd.Flags().StringToInt("node-config", nil, "[OPTIONAL] Number of vCPUs and disk size per node for the cluster, provided as key-value pairs. Arguments are num-cores=,disk-size-gb=,disk-iops= (AWS only). num-cores is required.") - createClusterCmd.Flags().MarkDeprecated("node-config", "use --region-info to specify num-cores, disk-size-gb, and disk-iops") createClusterCmd.Flags().StringArray("region-info", []string{}, `Region information for the cluster, provided as key-value pairs. Arguments are region=,num-nodes=,vpc=,num-cores=,disk-size-gb=,disk-iops= (AWS only). region, num-nodes, num-cores, disk-size-gb are required. Specify one --region-info flag for each region in the cluster.`) + createClusterCmd.MarkFlagRequired("region-info") createClusterCmd.Flags().String("preferred-region", "", "[OPTIONAL] The preferred region in a multi region cluster. The preferred region handles all read and write requests from clients.") createClusterCmd.Flags().String("default-region", "", "[OPTIONAL] The primary region in a partition-by-region cluster. The primary region is where all the tables not created in a tablespace reside.") } diff --git a/internal/client/client.go b/internal/client/client.go index c6175091..1919ec1c 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -38,7 +38,7 @@ import ( "golang.org/x/exp/slices" ) -// AuthApiClient is a auth YugabyteDB Aeon Client +// AuthApiClient is an auth YugabyteDB Aeon Client var cliVersion = "v0.1.0" @@ -57,9 +57,9 @@ func GetVersion() string { return cliVersion } -// NewAuthClient function is returning a new AuthApiClient Client +// NewAuthApiClient NewAuthClient function is returning a new AuthApiClient Client func NewAuthApiClient() (*AuthApiClient, error) { - url, err := ParseURL(viper.GetString("host")) + parseURL, err := ParseURL(viper.GetString("host")) if err != nil { logrus.Error(err) return nil, err @@ -71,7 +71,7 @@ func NewAuthApiClient() (*AuthApiClient, error) { logrus.Fatalln("No valid API key detected. Please run `ybm auth` to authenticate with YugabyteDB Aeon.") } - return NewAuthApiClientCustomUrlKey(url, apiKey) + return NewAuthApiClientCustomUrlKey(parseURL, apiKey) } func NewAuthApiClientCustomUrlKey(url *url.URL, apiKey string) (*AuthApiClient, error) { @@ -148,15 +148,11 @@ func (a *AuthApiClient) GetProjectID(projectID string) (string, error) { func (a *AuthApiClient) buildClusterSpec(cmd *cobra.Command, regionInfoList []map[string]string, regionNodeConfigsMap map[string][]ybmclient.NodeConfigurationResponseItem) (*ybmclient.ClusterSpec, error) { - var diskSizeGb int32 - var diskIops int32 - var memoryMb int32 var trackId string var trackName string - var regionInfoProvided bool var err error - clusterRegionInfo := []ybmclient.ClusterRegionInfo{} + var clusterRegionInfo []ybmclient.ClusterRegionInfo totalNodes := 0 regionNodeInfoMap := map[string]*ybmclient.OptionalClusterNodeInfo{} for _, regionInfo := range regionInfoList { @@ -174,7 +170,7 @@ func (a *AuthApiClient) buildClusterSpec(cmd *cobra.Command, regionInfoList []ma cloudInfo.SetCode(ybmclient.CloudEnum(cloudProvider)) } info := *ybmclient.NewClusterRegionInfo( - *ybmclient.NewPlacementInfo(cloudInfo, int32(regionNodes)), + *ybmclient.NewPlacementInfo(cloudInfo, regionNodes), ) if vpcName, ok := regionInfo["vpc"]; ok { vpcID, err := a.GetVpcIdByName(vpcName) @@ -218,15 +214,13 @@ func (a *AuthApiClient) buildClusterSpec(cmd *cobra.Command, regionInfoList []ma regionNodeInfoMap[region] = regionNodeInfo } - // This is to populate region in top level cloud info - region := "" regionCount := len(clusterRegionInfo) if regionCount > 0 { - regionInfoProvided = true - region = clusterRegionInfo[0].PlacementInfo.CloudInfo.Region if regionCount == 1 { clusterRegionInfo[0].SetIsDefault(true) } + } else { + return nil, fmt.Errorf("region info must be provided") } // For the default tier which is FREE, isProduction has to be false @@ -236,13 +230,10 @@ func (a *AuthApiClient) buildClusterSpec(cmd *cobra.Command, regionInfoList []ma if cmd.Flags().Changed("new-name") { clusterName, _ = cmd.Flags().GetString("new-name") } - cloudInfo := *ybmclient.NewCloudInfoWithDefaults() + cloud := "" if cmd.Flags().Changed("cloud-provider") { cloudProvider, _ := cmd.Flags().GetString("cloud-provider") - cloudInfo.SetCode(ybmclient.CloudEnum(cloudProvider)) - } - if regionInfoProvided { - cloudInfo.SetRegion(region) + cloud = cloudProvider } clusterInfo := *ybmclient.NewClusterInfoWithDefaults() @@ -267,7 +258,7 @@ func (a *AuthApiClient) buildClusterSpec(cmd *cobra.Command, regionInfoList []ma } if cmd.Flags().Changed("preferred-region") { preferredRegion, _ := cmd.Flags().GetString("preferred-region") - if clusterInfo.GetFaultTolerance() != ybmclient.ClusterFaultTolerance("REGION") { + if clusterInfo.GetFaultTolerance() != ("REGION") { return nil, fmt.Errorf("preferred region is allowed only for regional level fault tolerance") } @@ -302,115 +293,78 @@ func (a *AuthApiClient) buildClusterSpec(cmd *cobra.Command, regionInfoList []ma clusterInfo.SetClusterType(ybmclient.ClusterType(clusterType)) } - cloud := string(cloudInfo.GetCode()) tier := string(clusterInfo.GetClusterTier()) - if regionInfoProvided { - geoPartitioned := clusterInfo.GetClusterType() == "GEO_PARTITIONED" - clusterNodeInfoWithDefaults := *ybmclient.NewClusterNodeInfoWithDefaults() - // Create slice of desired regions. - regions := make([]string, 0, len(regionNodeInfoMap)) - for k, _ := range regionNodeInfoMap { - regions = append(regions, k) - } + geoPartitioned := clusterInfo.GetClusterType() == "GEO_PARTITIONED" + clusterNodeInfoWithDefaults := *ybmclient.NewClusterNodeInfoWithDefaults() + // Create slice of desired regions. + regions := make([]string, 0, len(regionNodeInfoMap)) + for k := range regionNodeInfoMap { + regions = append(regions, k) + } - if regionNodeConfigsMap == nil { - // Grab available node configurations by region. - regionNodeConfigsMap = a.GetSupportedNodeConfigurationsV2(cloud, tier, regions, geoPartitioned) - } + if regionNodeConfigsMap == nil { + // Grab available node configurations by region. + regionNodeConfigsMap = a.GetSupportedNodeConfigurationsV2(cloud, tier, regions, geoPartitioned) + } - // Create slice of region keys of node configurations response. - nodeConfigurationsRegions := make([]string, 0, len(regionNodeConfigsMap)) - for k, _ := range regionNodeConfigsMap { - nodeConfigurationsRegions = append(nodeConfigurationsRegions, k) + // Create slice of region keys of node configurations response. + nodeConfigurationsRegions := make([]string, 0, len(regionNodeConfigsMap)) + for k := range regionNodeConfigsMap { + nodeConfigurationsRegions = append(nodeConfigurationsRegions, k) + } + // For each desired region, grab appropriate node configuration and set node info. + for _, r := range regions { + var nodeConfigs []ybmclient.NodeConfigurationResponseItem + if slices.Contains(nodeConfigurationsRegions, r) { + nodeConfigs = regionNodeConfigsMap[r] + } else { + // Requested region not found in node configurations map. + // In this case, the map key is a string of all (comma-separated) regions, + // and the value is a list of node configurations that are available in all regions. + // So, we just use look through the first map value to find a node configuration to use. + nodeConfigs = regionNodeConfigsMap[nodeConfigurationsRegions[0]] } - // For each desired region, grab appropriate node configuration and set node info. - for _, r := range regions { - nodeConfigs := []ybmclient.NodeConfigurationResponseItem{} - if slices.Contains(nodeConfigurationsRegions, r) { - nodeConfigs = regionNodeConfigsMap[r] - } else { - // Requested region not found in node configurations map. - // In this case, the map key is a string of all (comma-separated) regions, - // and the value is a list of node configurations that are available in all regions. - // So, we just use look through the first map value to find a node configuration to use. - nodeConfigs = regionNodeConfigsMap[nodeConfigurationsRegions[0]] - } - requestedNodeInfo := regionNodeInfoMap[r] - requestedNumCores := requestedNodeInfo.GetNumCores() - userProvidedNumCores := requestedNumCores != 0 + requestedNodeInfo := regionNodeInfoMap[r] + requestedNumCores := requestedNodeInfo.GetNumCores() + userProvidedNumCores := requestedNumCores != 0 - var nodeConfig *ybmclient.NodeConfigurationResponseItem = nil - if !userProvidedNumCores { - requestedNumCores = clusterNodeInfoWithDefaults.GetNumCores() - } - for i, nc := range nodeConfigs { - if nc.GetNumCores() == requestedNumCores { - nodeConfig = &nodeConfigs[i] - break - } - } - if nodeConfig == nil { - logrus.Fatalf("No instance type found with %d cores in region %s\n", requestedNumCores, r) - } - regionNodeInfoMap[r].SetNumCores(nodeConfig.GetNumCores()) - regionNodeInfoMap[r].SetMemoryMb(nodeConfig.GetMemoryMb()) - if requestedNodeInfo.GetDiskSizeGb() == 0 { - // User did not specify a disk size. Default to included disk size. - regionNodeInfoMap[r].SetDiskSizeGb(nodeConfig.GetIncludedDiskSizeGb()) - } + var nodeConfig *ybmclient.NodeConfigurationResponseItem = nil + if !userProvidedNumCores { + requestedNumCores = clusterNodeInfoWithDefaults.GetNumCores() } - // Set per-region node info and cluster node info. - var currRegionNodeInfo *ybmclient.OptionalClusterNodeInfo = nil - for i, regionInfo := range clusterRegionInfo { - r := regionInfo.GetPlacementInfo().CloudInfo.Region - clusterRegionInfo[i].SetNodeInfo(*regionNodeInfoMap[r]) - logrus.Debugf("region=%s, node-info=%v\n", r, clusterRegionInfo[i].GetNodeInfo()) - if currRegionNodeInfo != nil && !geoPartitioned && *currRegionNodeInfo != clusterRegionInfo[i].GetNodeInfo() { - // Asymmetric node configurations are only allowed for geo-partitioned clusters. - logrus.Fatalln("Synchronous cluster regions must have identical node configurations") + for i, nc := range nodeConfigs { + if nc.GetNumCores() == requestedNumCores { + nodeConfig = &nodeConfigs[i] + break } - currRegionNodeInfo = (&clusterRegionInfo[i]).NodeInfo.Get() } - clusterInfo.SetNodeInfo(ToClusterNodeInfo(regionNodeInfoMap[regions[0]])) - } else { - clusterInfo.SetNodeInfo(*ybmclient.NewClusterNodeInfoWithDefaults()) - if cmd.Flags().Changed("node-config") { - nodeConfig, _ := cmd.Flags().GetStringToInt("node-config") - numCores := nodeConfig["num-cores"] - clusterInfo.NodeInfo.Get().SetNumCores(int32(numCores)) - if diskSize, ok := nodeConfig["disk-size-gb"]; ok { - diskSizeGb = int32(diskSize) - } - if diskIopsInt, ok := nodeConfig["disk-iops"]; ok { - diskIops = int32(diskIopsInt) - } - } - region = cloudInfo.GetRegion() - numCores := clusterInfo.NodeInfo.Get().GetNumCores() - memoryMb, err = a.GetFromInstanceType("memory", cloud, tier, region, numCores) - if err != nil { - return nil, err + if nodeConfig == nil { + logrus.Fatalf("No instance type found with %d cores in region %s\n", requestedNumCores, r) } - clusterInfo.NodeInfo.Get().SetMemoryMb(memoryMb) - - // Computing the default disk size if it is not provided - if diskSizeGb == 0 { - diskSizeGb, err = a.GetFromInstanceType("disk", cloud, tier, region, numCores) - if err != nil { - return nil, err - } + regionNodeInfoMap[r].SetNumCores(nodeConfig.GetNumCores()) + regionNodeInfoMap[r].SetMemoryMb(nodeConfig.GetMemoryMb()) + if requestedNodeInfo.GetDiskSizeGb() == 0 { + // User did not specify a disk size. Default to included disk size. + regionNodeInfoMap[r].SetDiskSizeGb(nodeConfig.GetIncludedDiskSizeGb()) } - clusterInfo.NodeInfo.Get().SetDiskSizeGb(diskSizeGb) - - if diskIops > 0 { - clusterInfo.NodeInfo.Get().SetDiskIops(diskIops) + } + // Set per-region node info and cluster node info. + var currRegionNodeInfo *ybmclient.OptionalClusterNodeInfo = nil + for i, regionInfo := range clusterRegionInfo { + r := regionInfo.GetPlacementInfo().CloudInfo.Region + clusterRegionInfo[i].SetNodeInfo(*regionNodeInfoMap[r]) + logrus.Debugf("region=%s, node-info=%v\n", r, clusterRegionInfo[i].GetNodeInfo()) + if currRegionNodeInfo != nil && !geoPartitioned && *currRegionNodeInfo != clusterRegionInfo[i].GetNodeInfo() { + // Asymmetric node configurations are only allowed for geo-partitioned clusters. + logrus.Fatalln("Synchronous cluster regions must have identical node configurations") } + currRegionNodeInfo = (&clusterRegionInfo[i]).NodeInfo.Get() } if cmd.Flags().Changed("default-region") { defaultRegion, _ := cmd.Flags().GetString("default-region") - if clusterInfo.GetClusterType() != ybmclient.ClusterType("GEO_PARTITIONED") { + if clusterInfo.GetClusterType() != ("GEO_PARTITIONED") { return nil, fmt.Errorf("default region is allowed only for geo partitioned clusters") } @@ -440,10 +394,7 @@ func (a *AuthApiClient) buildClusterSpec(cmd *cobra.Command, regionInfoList []ma clusterName, clusterInfo, softwareInfo) - clusterSpec.SetCloudInfo(cloudInfo) - if regionInfoProvided { - clusterSpec.SetClusterRegionInfo(clusterRegionInfo) - } + clusterSpec.SetClusterRegionInfo(clusterRegionInfo) return clusterSpec, nil } @@ -462,17 +413,6 @@ func (a *AuthApiClient) EditClusterSpec(cmd *cobra.Command, regionInfoList []map return a.buildClusterSpec(cmd, regionInfoList, regionNodeConfigsMap) } -func ToClusterNodeInfo(opt *ybmclient.OptionalClusterNodeInfo) ybmclient.ClusterNodeInfo { - clusterNodeInfo := *ybmclient.NewClusterNodeInfoWithDefaults() - clusterNodeInfo.SetNumCores(opt.GetNumCores()) - clusterNodeInfo.SetMemoryMb(opt.GetMemoryMb()) - clusterNodeInfo.SetDiskSizeGb(opt.GetDiskSizeGb()) - if iops, _ := opt.GetDiskIopsOk(); iops != nil { - clusterNodeInfo.SetDiskIops(*iops) - } - return clusterNodeInfo -} - func (a *AuthApiClient) GetInfo(providedAccountID string, providedProjectID string) { var err error a.AccountID, err = a.GetAccountID(providedAccountID) @@ -518,13 +458,13 @@ func (a *AuthApiClient) GetDrByName(drName string) (ybmclient.XClusterDrData, er } } - return ybmclient.XClusterDrData{}, fmt.Errorf("Could not get data for the DR config %s", drName) + return ybmclient.XClusterDrData{}, fmt.Errorf("could not get data for the DR config %s", drName) } func (a *AuthApiClient) ExtractProviderFromClusterName(clusterId string) ([]string, error) { clusterResp, _, err := a.GetCluster(clusterId).Execute() clusterData := clusterResp.GetData() - providers := []string{} + var providers []string if err != nil { return nil, err } @@ -593,7 +533,7 @@ func (a *AuthApiClient) GetDrDetailsByName(drName string) (ybmclient.XClusterDrI return drData.GetInfo(), nil } - return ybmclient.XClusterDrInfo{}, fmt.Errorf("Could not get data for the DR config %s", drName) + return ybmclient.XClusterDrInfo{}, fmt.Errorf("could not get data for the DR config %s", drName) } func (a *AuthApiClient) CreateCluster() ybmclient.ApiCreateClusterRequest { @@ -682,7 +622,7 @@ func (a *AuthApiClient) UpdateDbAuditExporterConfig(clusterId string, integratio } func (a *AuthApiClient) CreatePrivateServiceEndpointRegionSpec(regionArnMap map[string][]string) []ybmclient.PrivateServiceEndpointRegionSpec { - pseSpecs := []ybmclient.PrivateServiceEndpointRegionSpec{} + var pseSpecs []ybmclient.PrivateServiceEndpointRegionSpec for regionId, arnList := range regionArnMap { local := *ybmclient.NewPrivateServiceEndpointRegionSpec(arnList) @@ -693,7 +633,7 @@ func (a *AuthApiClient) CreatePrivateServiceEndpointRegionSpec(regionArnMap map[ } func (a *AuthApiClient) CreatePrivateServiceEndpointSpec(regionArnMap map[string][]string) []ybmclient.PrivateServiceEndpointSpec { - pseSpecs := []ybmclient.PrivateServiceEndpointSpec{} + var pseSpecs []ybmclient.PrivateServiceEndpointSpec for regionId, arnList := range regionArnMap { regionSpec := *ybmclient.NewPrivateServiceEndpointRegionSpec(arnList) @@ -1072,9 +1012,9 @@ func (a *AuthApiClient) ListSystemRbacRolesWithPermissions() ybmclient.ApiListRb func (a *AuthApiClient) CreateRoleSpec(cmd *cobra.Command, name string, permissionsMap map[string][]string) (*ybmclient.RoleSpec, error) { - rolePermissions := []ybmclient.ResourcePermissionInfo{} + var rolePermissions []ybmclient.ResourcePermissionInfo for resource, ops := range permissionsMap { - operationGroups := []ybmclient.ResourceOperationGroup{} + var operationGroups []ybmclient.ResourceOperationGroup for _, op := range ops { operationGroups = append(operationGroups, *ybmclient.NewResourceOperationGroup(ybmclient.ResourceOperationGroupEnum(op))) @@ -1272,7 +1212,7 @@ func (a *AuthApiClient) ListAccountUsers() ybmclient.ApiListAccountUsersRequest } func (a *AuthApiClient) CreateBatchInviteUserSpec(email string, roleId string) (*ybmclient.BatchInviteUserSpec, error) { - users := []ybmclient.InviteUserSpec{} + var users []ybmclient.InviteUserSpec user := *ybmclient.NewInviteUserSpecWithDefaults() user.SetEmail(email) @@ -1350,7 +1290,7 @@ func (a *AuthApiClient) WaitForTaskCompletionCI(entityId string, entityType ybmc return "", fmt.Errorf("receive interrupt signal, operation could still be on-going") case <-checkEveryInSec: apiRequest := a.ListTasks().TaskType(taskType).ProjectId(a.ProjectID).EntityId(entityId).Limit(1) - //Sometime the api do not need any entity type, for example VPC, VPC_PEERING + //Sometimes the api do not need any entity type, for example VPC, VPC_PEERING if len(entityType) > 0 { apiRequest.EntityType(entityType) } @@ -1416,7 +1356,7 @@ func (a *AuthApiClient) WaitForTaskCompletionFull(entityId string, entityType yb return "", fmt.Errorf("receive interrupt signal, operation could still be on-going") case <-checkEveryInSec: apiRequest := a.ListTasks().TaskType(taskType).ProjectId(a.ProjectID).EntityId(entityId).Limit(1) - //Sometime the api do not need any entity type, for example VPC, VPC_PEERING + //Sometimes the api do not need any entity type, for example VPC, VPC_PEERING if len(entityType) > 0 { apiRequest.EntityType(entityType) }