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

Add gcp provider support #141

Merged
merged 7 commits into from
Dec 14, 2023
Merged

Conversation

praveenkumar
Copy link
Member

No description provided.

compute.FirewallAllowArgs{
Protocol: pulumi.String("tcp"),
Ports: pulumi.ToStringArray([]string{
"22", "443", "6443", "80",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we create some common pkg with this information, so each provider will get values from there and will create the resources it need based on those values... (aws sgs, gcp firewall...).

Otherwise if for whatever reason we want to add some other port in the future we need to go provider by provider

Copy link
Member Author

Choose a reason for hiding this comment

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

we can make port as part of constants and use it for each provider.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated

return nil
}

func createKey(ctx *pulumi.Context) (*tls.PrivateKey, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

same here move to common pkg and reuse from all providers

Copy link
Member Author

Choose a reason for hiding this comment

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

for aws it is different signature func createKey(ctx *pulumi.Context) (*tls.PrivateKey, *ec2.KeyPair, error) but yes some of it can be consumed for each provider.

Copy link
Member Author

Choose a reason for hiding this comment

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

updated.

func (a *Provider) GetPlugin() *providerAPI.PluginInfo {
return &providerAPI.PluginInfo{
Name: "gcp",
Version: "v6.65.0"}
Copy link
Contributor

Choose a reason for hiding this comment

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

need this version at Containerfile as the other plugins
(i.e.

ARG PULUMI_AZURE_NATIVE_VERSION=v2.6.0
)

then also add the plugin installation cmd
(i.e.

pulumi plugin install resource azure-native ${PULUMI_AZURE_NATIVE_VERSION}
)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ack, will update that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@praveenkumar praveenkumar force-pushed the gcp_provider branch 5 times, most recently from c98de2d to 55e98ce Compare September 15, 2023 16:41
@adrianriobo
Copy link
Contributor

Can we add some README content on how to use the gcp, some code snippet on how to invoke the container or the binary? Envs required....

Also did you give a try to build a container and run from it AFAIK we will need to add the gcp cli to the container as well...may at https://github.com/crc-org/crc-cloud/blob/main/requirements.txt

@praveenkumar praveenkumar force-pushed the gcp_provider branch 4 times, most recently from 407c366 to a4c1e12 Compare December 13, 2023 07:36
@praveenkumar
Copy link
Member Author

Can we add some README content on how to use the gcp, some code snippet on how to invoke the container or the binary? Envs required....

Yes I updated the README which have all the steps and usecase.

Also did you give a try to build a container and run from it AFAIK we will need to add the gcp cli to the container as well...may at https://github.com/crc-org/crc-cloud/blob/main/requirements.txt

gcp cli added as part of container image, I am not sure why requirements.txt is there ?

@adrianriobo
Copy link
Contributor

gcp cli added as part of container image, I am not sure why requirements.txt is there ?

need to add both the pulumi plugin for gpc and the gcp cli I only see the the pulumi plugin, also that requirements is a way to manage tooling through pip if I am not wrong, so maybe you can just add a new line for the gcp cli there. AFAIK the benefit to handle it within the requirements.txt is that renovate acts on it out of the box.

@praveenkumar
Copy link
Member Author

need to add both the pulumi plugin for gpc and the gcp cli I only see the the pulumi plugin

Why I need to add gcp cli, I created the image using this container and able to create cluster using this image.

that requirements is a way to manage tooling through pip if I am not wrong

But where are we using this requirements only for renovate?

In go lang `init` is a special function and we shouldn't use it in this
context even `Init` so better to use `Create` which is what we are
doing.
@adrianriobo
Copy link
Contributor

But where are we using this requirements only for renovate?

Not only for renovate we are using it as an unified way for pkg management (+ it is supported by renovate).

So pros is we have a unified way to handle pkgs... cons is aws cli v2 is not supported there...on qenvs I use a different approach for each cli.

@adrianriobo
Copy link
Contributor

As a side note I realize you are using gcp classic provisioner https://github.com/pulumi/pulumi-gcp recently pulumi shift the way they create the provisioners to a native way using the cloud providers API https://github.com/pulumi/pulumi-google-native

It seems pulumi recommendation is to move to native provisioners...but they are not mature enough as an example I was able to use the azure native but not the aws as it misses still lot of resources... so add this as a comment we may can create a following issue as spike to migrate the gcp to the native provisioner WDYT?

This patch will remove the use of getSupportedProviders() and able
to use getProvider functionality for provider specific parameter.
```
$ crc-cloud create gcp --gcp-image-id crc --backed-url file:///tmp/gcp/ --project-name prkumar-test --output /tmp/gcp --key-filepath /home/prkumar/.crc/cache/crc_libvirt_4.13.6_amd64/id_ecdsa_crc --pullsecret-filepath /home/prkumar/pull-secret  --gcp-disk-size 100 --gcp-instance-type n1-standard-8
```
Right now it only contain the ports which is used across provider but we
should add more common constants to this package in future.
It is going to used for all the provider so better to extract it and
have different subpackage.
@praveenkumar
Copy link
Member Author

It seems pulumi recommendation is to move to native provisioners...but they are not mature enough as an example I was able to use the azure native but not the aws as it misses still lot of resources... so add this as a comment we may can create a following issue as spike to migrate the gcp to the native provisioner WDYT?

As per https://github.com/pulumi/pulumi-google-native looks like it is not stable and development is in active state so better to have follow up issue in case we want to switch over in near future.

@adrianriobo adrianriobo self-requested a review December 14, 2023 11:13
@adrianriobo adrianriobo merged commit f4868b9 into crc-org:main Dec 14, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants