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

feat(instance): support snapshot based instance #3787

Merged
merged 5 commits into from
Jul 1, 2024

Conversation

tormath1
Copy link
Contributor

@tormath1 tormath1 commented Apr 23, 2024

Hi,

In this PR I tried to allow the CLI to deploy an instance directly from a snapshot without calling the marketplace API.

scw instance server \
  create image=none \
  root-volume=l:11111...11 \
  cloud-init=@/tmp/config.json

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request.
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Release note for CHANGELOG:

NONE

@remyleone remyleone added the instance Instance issues, bugs and feature requests label Apr 23, 2024
serverType *instance.ServerType
)
if args.Image != "none" {
getImageResponse, err := apiInstance.GetImage(&instance.GetImageRequest{
Copy link
Member

Choose a reason for hiding this comment

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

getImageResponse is shadowing the one created in previous context.

Suggested change
getImageResponse, err := apiInstance.GetImage(&instance.GetImageRequest{
var err error
getImageResponse, err = apiInstance.GetImage(&instance.GetImageRequest{


serverType := getServerType(apiInstance, serverReq.Zone, serverReq.CommercialType)
serverType := getServerType(apiInstance, serverReq.Zone, serverReq.CommercialType)
Copy link
Member

Choose a reason for hiding this comment

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

serverType shadows the one created in previous context

Suggested change
serverType := getServerType(apiInstance, serverReq.Zone, serverReq.CommercialType)
serverType = getServerType(apiInstance, serverReq.Zone, serverReq.CommercialType)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops thanks a lot. Missed that. That should explain the failing CI.

@tormath1 tormath1 force-pushed the tormath1/instance-snapshot branch from 7f521e5 to ad04273 Compare May 21, 2024 14:24
@remyleone remyleone added the priority:medium Improvements that are not the main priority label Jun 4, 2024
@tormath1 tormath1 requested a review from remyleone as a code owner June 12, 2024 11:51
@codecov-commenter
Copy link

codecov-commenter commented Jun 13, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 69.25%. Comparing base (5bf53e0) to head (3073489).
Report is 128 commits behind head on master.

Files Patch % Lines
...nal/namespaces/instance/v1/custom_server_create.go 83.33% 4 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (5bf53e0) and HEAD (3073489). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (5bf53e0) HEAD (3073489)
2 1
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3787      +/-   ##
==========================================
- Coverage   75.45%   69.25%   -6.21%     
==========================================
  Files         202      288      +86     
  Lines       44323    54603   +10280     
==========================================
+ Hits        33444    37814    +4370     
- Misses       9653    15256    +5603     
- Partials     1226     1533     +307     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tormath1 and others added 3 commits June 13, 2024 18:48
This PR allow users to create an instance from a snapshot directly
without creating an image.
It's already supported by the APIs.

Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
@remyleone remyleone changed the title fix(instance): support snapshot based instance feat(instance): support snapshot based instance Jun 14, 2024
@tormath1
Copy link
Contributor Author

@remyleone do you want me to rebase on master?

@remyleone remyleone enabled auto-merge July 1, 2024 09:39
@remyleone remyleone added this pull request to the merge queue Jul 1, 2024
@remyleone
Copy link
Member

Thanks a lot @tormath1 for your contribution :)

Merged via the queue into scaleway:master with commit bf761bf Jul 1, 2024
12 checks passed
@tormath1 tormath1 deleted the tormath1/instance-snapshot branch July 1, 2024 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
instance Instance issues, bugs and feature requests priority:medium Improvements that are not the main priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants