Skip to content

feat(instance): support snapshot based instance#3787

Merged
remyleone merged 5 commits intoscaleway:masterfrom
tormath1:tormath1/instance-snapshot
Jul 1, 2024
Merged

feat(instance): support snapshot based instance#3787
remyleone merged 5 commits intoscaleway:masterfrom
tormath1:tormath1/instance-snapshot

Conversation

@tormath1
Copy link
Copy Markdown
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
@tormath1 tormath1 force-pushed the tormath1/instance-snapshot branch from bdc9f1a to 7f521e5 Compare April 24, 2024 09:53
serverType *instance.ServerType
)
if args.Image != "none" {
getImageResponse, err := apiInstance.GetImage(&instance.GetImageRequest{
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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 force-pushed the tormath1/instance-snapshot branch from ad04273 to 7ea13bb Compare June 12, 2024 11:51
@tormath1 tormath1 requested a review from remyleone as a code owner June 12, 2024 11:51
@codecov-commenter
Copy link
Copy Markdown

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 force-pushed the tormath1/instance-snapshot branch from 082db02 to fc68b94 Compare June 13, 2024 16:49
@remyleone remyleone changed the title fix(instance): support snapshot based instance feat(instance): support snapshot based instance Jun 14, 2024
@tormath1
Copy link
Copy Markdown
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
Copy Markdown
Member

Thanks a lot @tormath1 for your contribution :)

Merged via the queue into scaleway:master with commit bf761bf Jul 1, 2024
@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