Skip to content

feat(instance): add support for bootscript #652

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Oct 26, 2020
Merged

feat(instance): add support for bootscript #652

merged 7 commits into from
Oct 26, 2020

Conversation

@remyleone
Copy link
Member

No description provided.

@remyleone remyleone linked an issue Oct 21, 2020 that may be closed by this pull request
Add support for bootscripts to the scaleway_instance_server kind of resource #523
Closed
kindermoumoute
kindermoumoute requested changes Oct 21, 2020
View reviewed changes
@Sh4d1
Copy link
Contributor

Sh4d1 commented Oct 21, 2020

What happens when a user specifies a bootscript without changing the boot type ? We could either throw an error, or change the boot type.

kindermoumoute
kindermoumoute requested changes Oct 21, 2020
View reviewed changes
Sh4d1
Sh4d1 suggested changes Oct 21, 2020
View reviewed changes
Copy link
Contributor

@Sh4d1 Sh4d1 left a comment

Choose a reason for hiding this comment

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

Need to set the bootype in create req event when no bootscript

Some questions:

  • what about the update?
  • what if the user sets the boot_type to local and a bootscript ID ? The states will mismatch, I think throwing an error would be better

@NoSuchCommand
Copy link

NoSuchCommand commented Oct 21, 2020 •
edited
Loading

Need to set the bootype in create req event when no bootscript

Some questions:

* what about the update?

* what if the user sets the boot_type to local and a bootscript ID ? The states will mismatch, I think throwing an error would be better

A boot_type set to local and a bootscript are not mutually exclusive. This is actually the current behaviour of the API : a new instance inherits the default bootscript of the AZ, no matter whether its boot_type is set to local or not.

The user might want to use local boot and have a bootscript of her choice already set, just in case.

Conclusion: this should not throw an error.

@NoSuchCommand NoSuchCommand reopened this Oct 21, 2020
kindermoumoute
kindermoumoute requested changes Oct 26, 2020
View reviewed changes
kindermoumoute
kindermoumoute reviewed Oct 26, 2020
View reviewed changes
kindermoumoute
kindermoumoute previously approved these changes Oct 26, 2020
View reviewed changes
Copy link
Contributor

@kindermoumoute kindermoumoute left a comment

Choose a reason for hiding this comment

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

LGTM

Sh4d1
Sh4d1 approved these changes Oct 26, 2020
View reviewed changes
Copy link
Contributor

@Sh4d1 Sh4d1 left a comment

Choose a reason for hiding this comment

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

LGTM

@remyleone remyleone merged commit 5a71eff into scaleway:master Oct 26, 2020
@remyleone remyleone deleted the bootscript branch October 26, 2020 14:55
@remyleone remyleone added the instance Instance issues, bugs and feature requests label Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kindermoumoute kindermoumoute kindermoumoute left review comments

+1 more reviewer

@Sh4d1 Sh4d1 Sh4d1 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

instance Instance issues, bugs and feature requests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add support for bootscripts to the scaleway_instance_server kind of resource

4 participants

@remyleone @Sh4d1 @NoSuchCommand @kindermoumoute