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

Imprv/389 #413

Open
wants to merge 2 commits into
base: release/1.9.1
Choose a base branch
from
Open

Conversation

grdavies
Copy link

Added ability to provide sysprep/cloud-init via variable instead of just an input script

Copy link
Collaborator

@Gevorg-Khachatryan-97 Gevorg-Khachatryan-97 left a comment

Choose a reason for hiding this comment

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

reviewed

@bhati-pradeep bhati-pradeep changed the base branch from main to release/1.9.1 October 25, 2023 10:28
- Absolute file path to the script.
type: path
required: true
suboptions:
Copy link
Collaborator

Choose a reason for hiding this comment

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

suboptions is already defined above.

@@ -355,14 +355,17 @@ def _build_spec_boot_config(self, payload, param):
return payload, None

def _build_spec_gc(self, payload, param):
fpath = param["script_path"]
if param["script_path"]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

what if script path is not given?

Suggested change
if param["script_path"]:
if param.get("script_path"):

with open(fpath, "rb") as f:
content = base64.b64encode(f.read())
else:
content = base64.b64encode(param["script"])
Copy link
Collaborator

@Gevorg-Khachatryan-97 Gevorg-Khachatryan-97 Oct 26, 2023

Choose a reason for hiding this comment

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

param of b64encode function should be a bytes-like object

Suggested change
content = base64.b64encode(param["script"])
content = base64.b64encode(param["script"].encode('ascii'))

@bhati-pradeep
Copy link
Collaborator

@grdavies Please add tests for your change. @alaa-bish can guide you here.

@bhati-pradeep
Copy link
Collaborator

@grdavies Can you please look into the PR comments in order to make it to release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.9.3_triage Bug fixes & Improvements
Projects
Development

Successfully merging this pull request may close these issues.

[Imprv] add functionality to provide sysprep or cloud-init to module ntnx_vms via a variable instead of a file
3 participants