convert vm specs to provider specific instance type name by anjannath · Pull Request #277 · redhat-developer/mapt

anjannath · 2024-08-12T17:56:01Z

fixes #281

adrianriobo · 2024-08-14T08:37:05Z

+				if viper.GetString(params.LinuxArch) == "arm64" {
+					arch = resources.Arm64
+				}
+				ec2Types, _ = resources.GetAwsMachineTypes(viper.GetInt32(params.CPUs), viper.GetInt32(params.Memory), arch)


This logic needs to be moved to the pkg; inside the actions...

the rational behind... is at the moment the interface to interact with mapt is through cmd, but in case wanna support the k8s controller model it will directly use the logic on actions as so we need to accept the requested features there and search for the type of machine there

so iiuc the required cpus and memories should be part of the Request struct on actions and these will be what passed from the cmd layer and pkg will figure out the instance types based on the cpus,memory and arch

yeah, probably the search module will have an struct with those values you use to query (CPU, Ram, Nested Virtualization, GPU and arch) that struct will be part of the request struct

key part any logic should be encapsulated within actions, then cmd or controllers just fill the request structs there

added CPUs and Memory fields to the Request struct itself, and functions to fetch the instance types

but as you suggest encapsulating these in a new request structs would be better, then we can have methods on it to fetch the instance types.. e.g named AwsInstanceTypeRequest AzureInstanceTypeRequest

adrianriobo · 2024-09-16T08:25:09Z

+	return fmt.Errorf("Invalid values for CPUs: %d, Memory: %d and Arch: %s", cpus, memory, arch)
+}
+
+func (r *AwsInstanceRequest) GetMachineTypes() ([]string, error) {


May I would suggest change a bit the structure of the pkg here, I would create a interface for

func GetMachineTypes() ([]string, error) {

And would be implemented by azure and aws, each specific code would be on its own file i.e:

instancesTypes, instancesTypesAzure, instancesTypesAWS, probably for the interface approach you would require an extra module on the same pkg (api) similar to https://github.com/adrianriobo/goax/tree/main/pkg/goax/app

does this make sense to you? I think it would organize the functions a bit better as right now they are kind of mixed on instancesTypes, and eventually if we add gpc support it would add more code on same file

yeah, i can try this approach will update the PR

adrianriobo · 2024-09-16T08:27:44Z

 	flagSet.Bool(spot, false, spotDesc)
 	flagSet.Bool(amiKeepCopy, false, amiKeepCopyDesc)
 	flagSet.AddFlagSet(params.GetGHActionsFlagset())
+	flagSet.AddFlagSet(params.GetCpusAndMemoryFlagset())


Here paras are added but not process of the data?

ahh yes, this is missing the corresponding code to make use of the flags, will fix

adrianriobo · 2024-09-18T10:16:42Z

+	EncryptionAtHostSupported    bool
+}
+
+func (vm *virtualMachine) NestedVirtSupported() bool {


Are any of this functions public?

yeah the functions are public but only used in the same file, those could be made private, is that your suggestion to make them private?

i was also thinking maybe this file could be in pkg/provider/azure/instancetypes.go instead

is that your suggestion to make them private?

Yeah, that it is

i was also thinking maybe this file could be in pkg/provider/azure/instancetypes.go instead

Yeah an option would be something like that or keep the api for the providers outside and each implementation under the provider, but for the time being it is fine for me to keep it here

okay, PR is updated now, PTALA :)

adrianriobo · 2024-09-18T13:46:07Z

Also I did not check this yet, I would like to know if is there any dependecy on this module from pulumi or is everything only sdk based.

I would like it sdk based ...otherwise if someone try to use may require the aws cli installed down the line, which may prevent for using this module.

anjannath · 2024-09-19T07:00:41Z

Also I did not check this yet, I would like to know if is there any dependecy on this module from pulumi or is everything only sdk based.

I would like it sdk based ...otherwise if someone try to use may require the aws cli installed down the line, which may prevent for using this module.

it uses only the SDK and no dependency on pulumi, here's a small code snippet to use the instancetypes module:

package main

import (
    "fmt"
    "github.com/redhat-developer/mapt/pkg/provider/util/instancetypes"
)

func main() {
    azureInstanceRequest := instancetypes.AzureInstanceRequest{
        CPUs: 4,
        MemoryGib: 8,
        NestedVirt: true,
        Arch: instancetypes.Arm64,
    }
    if vms, err := azureInstanceRequest.GetMachineTypes(); err == nil {
        for _, vm := range vms {
            fmt.Println(vm)
        }
    }
}

and the go.mod for it don't have any pulumi dependencies

 % cat go.mod
module machinenames

go 1.22.4

replace github.com/redhat-developer/mapt => ../../

require github.com/redhat-developer/mapt v0.0.0-00010101000000-000000000000

require (
        github.com/Azure/azure-sdk-for-go/sdk/azcore v1.13.0 // indirect
        github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.7.0 // indirect
        github.com/Azure/azure-sdk-for-go/sdk/internal v1.10.0 // indirect
        github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v6 v6.0.0 // indirect
        github.com/AzureAD/microsoft-authentication-library-for-go v1.2.2 // indirect
        github.com/atotto/clipboard v0.1.4 // indirect
        github.com/aws/amazon-ec2-instance-selector/v2 v2.4.2-0.20231006140257-d989c5d76f0e // indirect
        github.com/aws/aws-sdk-go v1.45.6 // indirect
        github.com/aws/aws-sdk-go-v2 v1.27.2 // indirect
        github.com/aws/aws-sdk-go-v2/config v1.27.18 // indirect
        github.com/aws/aws-sdk-go-v2/credentials v1.17.18 // indirect
        github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.5 // indirect
        github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.9 // indirect
        github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.9 // indirect
        github.com/aws/aws-sdk-go-v2/internal/ini v1.8.0 // indirect
        github.com/aws/aws-sdk-go-v2/service/ec2 v1.142.0 // indirect
        github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.11.2 // indirect
        github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.11.11 // indirect
        github.com/aws/aws-sdk-go-v2/service/pricing v1.21.6 // indirect
        github.com/aws/aws-sdk-go-v2/service/sso v1.20.11 // indirect
        github.com/aws/aws-sdk-go-v2/service/ssooidc v1.24.5 // indirect
        github.com/aws/aws-sdk-go-v2/service/sts v1.28.12 // indirect
        github.com/aws/smithy-go v1.20.2 // indirect
        github.com/aymanbagabas/go-osc52/v2 v2.0.1 // indirect
        github.com/blang/semver/v4 v4.0.0 // indirect
        github.com/charmbracelet/bubbles v0.16.1 // indirect
        github.com/charmbracelet/bubbletea v0.25.0 // indirect
        github.com/charmbracelet/lipgloss v0.7.1 // indirect
        github.com/containerd/console v1.0.4 // indirect
        github.com/evertras/bubble-table v0.15.2 // indirect
        github.com/golang-jwt/jwt/v5 v5.2.1 // indirect
        github.com/google/uuid v1.6.0 // indirect
        github.com/imdario/mergo v0.3.16 // indirect
        github.com/jmespath/go-jmespath v0.4.0 // indirect
        github.com/kylelemons/godebug v1.1.0 // indirect
        github.com/lucasb-eyer/go-colorful v1.2.0 // indirect
        github.com/mattn/go-isatty v0.0.20 // indirect
        github.com/mattn/go-localereader v0.0.1 // indirect
        github.com/mattn/go-runewidth v0.0.15 // indirect
        github.com/mitchellh/go-homedir v1.1.0 // indirect
        github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect
        github.com/muesli/cancelreader v0.2.2 // indirect
        github.com/muesli/reflow v0.3.0 // indirect
        github.com/muesli/termenv v0.15.2 // indirect
        github.com/oliveagle/jsonpath v0.0.0-20180606110733-2e52cf6e6852 // indirect
        github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
        github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c // indirect
        github.com/rivo/uniseg v0.4.7 // indirect
        github.com/sahilm/fuzzy v0.1.0 // indirect
        go.uber.org/multierr v1.11.0 // indirect
        golang.org/x/crypto v0.25.0 // indirect
        golang.org/x/net v0.27.0 // indirect
        golang.org/x/sync v0.7.0 // indirect
        golang.org/x/sys v0.22.0 // indirect
        golang.org/x/term v0.22.0 // indirect
        golang.org/x/text v0.16.0 // indirect
)

Signed-off-by: Anjan Nath <kaludios@gmail.com>

openshift-ci Bot added the do-not-merge/work-in-progress label Aug 12, 2024

anjannath force-pushed the spec-to-vmtypes branch 2 times, most recently from a0d52bd to 14d8432 Compare August 13, 2024 19:11

anjannath changed the title ~~[wip] convert vm specs to provider specific instance type name~~ convert vm specs to provider specific instance type name Aug 13, 2024

openshift-ci Bot removed the do-not-merge/work-in-progress label Aug 13, 2024

adrianriobo reviewed Aug 14, 2024

View reviewed changes

anjannath force-pushed the spec-to-vmtypes branch 6 times, most recently from b9aba0e to aa9fd25 Compare August 21, 2024 09:52

anjannath force-pushed the spec-to-vmtypes branch 4 times, most recently from 3b2ef23 to 7a2978b Compare August 27, 2024 08:09

anjannath mentioned this pull request Sep 2, 2024

Spot module as a separate package #284

Merged

anjannath force-pushed the spec-to-vmtypes branch 2 times, most recently from 1786c81 to 409f981 Compare September 3, 2024 09:41

adrianriobo reviewed Sep 16, 2024

View reviewed changes

anjannath force-pushed the spec-to-vmtypes branch from 409f981 to c1c16a1 Compare September 18, 2024 07:52

adrianriobo reviewed Sep 18, 2024

View reviewed changes

adrianriobo approved these changes Sep 19, 2024

View reviewed changes

convert vm specs to provider specific instance type name

51981df

Signed-off-by: Anjan Nath <kaludios@gmail.com>

anjannath force-pushed the spec-to-vmtypes branch from c1c16a1 to 51981df Compare September 19, 2024 07:19

adrianriobo self-requested a review September 19, 2024 07:23

adrianriobo approved these changes Sep 19, 2024

View reviewed changes

adrianriobo merged commit 0971b20 into redhat-developer:main Sep 19, 2024

adrianriobo mentioned this pull request Oct 3, 2024

[Bug] Missing params for search instance type #301

Closed

adrianriobo mentioned this pull request Oct 14, 2024

[BUG] Documentation is missing information on new parameters to set type of machine based on args #310

Closed

Conversation

anjannath commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adrianriobo commented Sep 18, 2024

Uh oh!

anjannath commented Sep 19, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

anjannath commented Aug 12, 2024 •

edited

Loading