Skip to content
This repository has been archived by the owner on Aug 21, 2024. It is now read-only.

add gcp image list command to CLI #37

Merged
merged 3 commits into from
Oct 14, 2022

Conversation

miyunari
Copy link
Member

@miyunari miyunari commented Oct 11, 2022

Hey @major @FKolwa ,

I am a bit confused right now. We assign the registered github secret secrets.GOOGLE_APPLICATION_CREDENTIALS to an environment variable GCP_APP_CREDENTIALS, but we never use it.

In the get_google_images function we create an compute_v1.ImagesClient client, is he supposed to use those credentials?

I went to the google docs, but it also didn't really helped me. :)

Can you give me some hints? 😅

closes #17

@F-X64
Copy link
Member

F-X64 commented Oct 11, 2022

@miyunari Yup that is correct! It isn't necessary to reference the ENV in code.
In fact you won't find any of the other secrets either (like AWS_ACCESS_KEY_ID).
Most (if not all) cloud provider CLIs authenticate using local configurations that are loaded into ENVs at runtime.
Usually you would use you user credentials to sign into Google Cloud but with an automated service like this a service account can be used.
The fact that "GOOGLE_APPLICATION_CREDENTIALS" is set for the workflow step means that the gcloud cli will be able to read it and use it to authenticate with the cloud provider API. I'll leave you a link in case you want to read more about Google Application Default Credentials (ADC): https://cloud.google.com/docs/authentication/provide-credentials-adc#local-key

A small hint though: The env needs to be mapped to "GOOGLE_APPLICATION_CREDENTIALS" in the workflow context as well for it to properly work.

Hope this helps a little!

.github/workflows/main.yml Outdated Show resolved Hide resolved
@miyunari miyunari force-pushed the feat/add_google_image_support_#17 branch from 4c0307c to e086b71 Compare October 11, 2022 08:10
@miyunari
Copy link
Member Author

Thank you @FKolwa ! That was exactly the information I was looking for 😄

But now, there is another issue 😅 Unfortunately it's now unclear to me, how to test my changes.
I got this reference from major, but I don't see the correlation:
https://github.com/redhatcloudx/rhelocator/blob/b97967e77354d0331f0f7bc4a607c00b4b1eea16/tests/test_cli.py#L77-L103

@F-X64
Copy link
Member

F-X64 commented Oct 11, 2022

@miyunari Haha yes this is a bit confusing tbh!

The gcloud implementation is pretty rough at this point. get_google_images currently queries all images in the rhel-cloud project and returns everything that isn't deprecated.

In the scope of this ticket, my requirement for an end to end test would be

  • query the correct api endpoint by calling gcp_images by using 'runner.invoke'
  • parse the json data
  • confirm that all images that are returned do not contain the first level key status with the value 'DEPRECATED'
    In this case you can copy / paste most of what @major wrote for the azure test!

For the offline test

  • copy the structure of the e2e test you just wrote
  • create a new mock for the google images in conftest (you can take a look at the AWS mockups. You need to create a new list of mocked images in a json format and create a new fixture that is passed to you offline test).

Now for the tricky part: How do you know what data structure to expect from the google API?
Well if you call gcloud and query for projects within 'rhel-cloud' you will receive something like this:

{
"architecture": "X86_64",
"archiveSizeBytes": "4184623872",
"creationTimestamp": "2022-09-20T16:32:45.492-07:00",
"description": "Red Hat, Red Hat Enterprise Linux, 9, x86_64 built on 20220920, supports Shielded VM features",
"diskSizeGb": "20",
"family": "rhel-9",
"guestOsFeatures": [
{
"type": "UEFI_COMPATIBLE"
},
{
"type": "VIRTIO_SCSI_MULTIQUEUE"
},
{
"type": "SEV_CAPABLE"
},
{
"type": "GVNIC"
}
],
"id": "2043557223711896434",
"kind": "compute#image",
"labelFingerprint": "42WmSpB8rSM=",
"licenseCodes": [
"7883559014960410759"
],
"licenses": [
"https://www.googleapis.com/compute/beta/projects/rhel-cloud/global/licenses/rhel-9-server"
],
"name": "rhel-9-v20220920",
"rawDisk": {
"containerType": "TAR",
"source": ""
},
"rolloutOverride": {
"defaultRolloutTime": "2022-09-25T15:32:42Z",
"locationRolloutPolicies": {
"zones/asia-east1-a": "2022-09-22T04:32:42Z",
....
"zones/us-west4-c": "2022-09-25T04:32:42Z"
}
},
"selfLink": "https://www.googleapis.com/compute/beta/projects/rhel-cloud/global/images/rhel-9-v20220920",
"sourceType": "RAW",
"status": "READY",
"storageLocations": [
"eu",
"asia",
"us"
]
}

At this point we don't extract any specific information from this returned data (like we do for AWS) and this is not within the scope of your ticket so feel free to create a minimal test mockup version of this data structure that only contains something like
{ "status": "READY" }

@major
Copy link
Member

major commented Oct 11, 2022

I am a bit confused right now. We assign the registered github secret secrets.GOOGLE_APPLICATION_CREDENTIALS to an environment variable GCP_APP_CREDENTIALS, but we never use it.

Oh boy, that was my mistake. I must have been looking at two things at the same time and put the wrong variable name in the actions workflow. 🤦🏻‍♂️

Sorry about that.

tests/test_cli.py Outdated Show resolved Hide resolved
@miyunari
Copy link
Member Author

miyunari commented Oct 13, 2022

Oh boy, that was my mistake. I must have been looking at two things at the same time and put the wrong variable name in the actions workflow. 🤦🏻‍♂️

@major I think you were right 😄 . I shortened the variable name, because I thought we have to store it in config.py and use it somewhere 🤦‍♀️

@FKolwa thank you for your detailed explanation! That really helped! 😄

tests/test_cli.py Outdated Show resolved Hide resolved
tests/conftest.py Show resolved Hide resolved
@miyunari miyunari force-pushed the feat/add_google_image_support_#17 branch from c3bed81 to 42bdb97 Compare October 13, 2022 11:09
@miyunari miyunari marked this pull request as ready for review October 13, 2022 11:11
@miyunari
Copy link
Member Author

Oh, I see I have some merge conflicts, will try to resolve

Signed-off-by: Janine Olear <pninak@web.de>
@miyunari miyunari force-pushed the feat/add_google_image_support_#17 branch 4 times, most recently from 2d29845 to 32bbc19 Compare October 13, 2022 11:37
Signed-off-by: Janine Olear <pninak@web.de>
@miyunari miyunari force-pushed the feat/add_google_image_support_#17 branch from 32bbc19 to 2a7ae17 Compare October 13, 2022 11:51
tests/test_cli.py Outdated Show resolved Hide resolved
Signed-off-by: Janine Olear <pninak@web.de>
Copy link
Member

@F-X64 F-X64 left a comment

Choose a reason for hiding this comment

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

Thanks for the implementation @miyunari ! Changes look great 👍

@miyunari miyunari mentioned this pull request Oct 14, 2022
@miyunari miyunari requested a review from major October 14, 2022 09:36
Copy link
Member

@major major left a comment

Choose a reason for hiding this comment

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

👍🏻

@major major merged commit c2e391b into redhatcloudx:main Oct 14, 2022
@miyunari miyunari deleted the feat/add_google_image_support_#17 branch October 14, 2022 12:29
@miyunari miyunari self-assigned this Nov 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI: Add Google images support
3 participants