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

builder: add deeplearning-platform-release project (issue #229) #230

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jbaldassari
Copy link

Similar to the issue/changes described in #192, this adds support for pulling source images from the deeplearning-platform-release project.

Closes #229

@jbaldassari jbaldassari requested a review from a team as a code owner July 24, 2024 20:52
Copy link

hashicorp-cla-app bot commented Jul 24, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@lbajolet-hashicorp
Copy link
Contributor

Hi @jbaldassari,

Thanks for the PR! I wonder though, I missed that option last time I changed/merged code into the driver regarding the default projects, and if this is a Google official project, I can see us adding it to the list (haven't been able to confirm that unfortunately, would appreciate if you could share a link to the GCE docs if that's the case?), that being said, there's the source_image_project_id argument you can use to specify extra projects to look images from, does that allow you to get the image in the current state?

Just trying to limit the amount of projects we add to our default list, as it is quite rigid to update, and the more projects we add to that list, the slower (potentially) searching for a specific image/image family may become.

That being said the code looks good to me, if there's a compelling reason to add this project to the default list, we should be able to merge this PR, and we have a release planned for next week, so we can definitely fold this in then.

@jbaldassari
Copy link
Author

jbaldassari commented Jul 26, 2024

Thanks for the info, @lbajolet-hashicorp ! I'm sorry we missed that source_image_project_id option when we were looking for workarounds. I've confirmed that setting source_image_project_id=deeplearning-platform-release does work for us in this case.

if this is a Google official project, I can see us adding it to the list (haven't been able to confirm that unfortunately, would appreciate if you could share a link to the GCE docs if that's the case?),

I think the confusion here is that these images are part of Deep Learning, which is a Google Cloud product, but these images are not part of GCE itself. The Deep Learning docs reference the deeplearning-platform-release project here and here.

That being said, since we have a workaround, I'll leave it up to you if you want to include this as one of the default projects or recommend people use source_image_project_id for less common cases like this.

@lbajolet-hashicorp
Copy link
Contributor

Don't be sorry @jbaldassari, I also missed it last time when merging the PR you referenced, so there's probably something we can do to make it more visible 😅

I'll keep the PR open for now, and bring that up next time we discuss open issues with the rest of the team, we'll make a decision then.

Thanks for the update, glad source_image_project_id did the trick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot use source images from deeplearning-platform-release project
2 participants