-
Couldn't load subscription status.
- Fork 7
Updating cloud-init config files and base image #24
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to update the bootstrap iso as well, since the logic there is to read from sr dev and look for exact file names of 99_network_config.cfg.
Also did you tested this in IBMi env?
Yes I tested PR in IBMI env, new cloud-init is working as expected. |
|
I was facing the issue that I talked about can you please take a look at the getconfig systemd log in bootstrap process? We have to fix that. |
@dharaneeshvrd this issue will be solved when we create bootstrap.iso with new steps, here is the new bootstrap iso file is kept which is used for testing -> http://9.114.99.227/fedora41-bootc-bootstrap.iso |
99c66be to
0a5daa2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adarshagrawal38
I think PartOf config will start all the systemd at the same time, but what we want is hierarichal start, I think that can be achieved by Requires and After config itself.
We just need to configure them properly such that each level depends on the previous level. Example if you take hmc-agent, agent and server services should be started only after vllm is started, all three services should not be started at the same time.
Can you please take a look at that? Also monitor the start time of the services that it follows one after the other.
base-image/base_config.sh
Outdated
|
|
||
| echo "base_config.sh run successfully!" | ||
|
|
||
| echo "base_config.sh run successfully!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the purpose of this script? It doesn't do any operations rt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah will remove this script.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please rename base_config to appropriate one like root_pim or something like that, it should denote that this act as a base service for all the systemd services that are getting installed as part of PIM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-naming it as pim_init.service
| @@ -0,0 +1,10 @@ | |||
| version: 2 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we keep the file with suffix .cfg as kept earlier?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In one of the solution I saw keeping network related information in network-config loads network for the machine.
I can try changing file name to network-config.cfg and see if it work. I think cfg extension is added for custom config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, keep whichever works.
| users: | ||
| - name: {{ config["ssh"]["user-name"] }} | ||
| sudo: ['ALL=(ALL) NOPASSWD:ALL'] | ||
| lock_passwd: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this cfg do?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Config is for loading user and ssh keys in the LPAR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, i asked about lock_passwd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is used to disable password login for pim user
| gateway4: {{ config["partition"]["network"]["ip"]["gateway"] }} | ||
| nameservers: | ||
| addresses: | ||
| - {{ config["partition"]["network"]["ip"]["nameserver"] }} No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add new line EOF
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
base-image/base_config.service
Outdated
|
|
||
| [Install] | ||
| WantedBy=cloud-config.target | ||
| WantedBy=cloud-config.target |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add newline here as well
3e78cdc to
772cd4e
Compare
Signed-off-by: Adarsh Agrawal <[email protected]>
772cd4e to
02f9e44
Compare
Signed-off-by: Adarsh Agrawal <[email protected]>
Signed-off-by: Adarsh Agrawal <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also we have to document that the user should follow the hierarchical structure we are following to keep the application grouped. Maybe in examples/README.md or builder-guide.md. Please explain it with the hmc-agent example itself.
base-image/Containerfile
Outdated
|
|
||
| RUN systemctl unmask base_config.service | ||
| RUN systemctl enable base_config.service | ||
| RUN systemctl unmask pim_init.service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its better to align with the image name, base-service
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kept it as base.service
base-image/base.service
Outdated
| @@ -5,7 +5,7 @@ Wants=network-online.target cloud-config.target | |||
|
|
|||
| [Service] | |||
| Type=oneshot | |||
| ExecStart=/usr/bin/env /bin/bash /usr/bin/base_config.sh | |||
| ExecStart=/bin/true | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's print some logs, so that we can get the timestamp in journalctl log, not sure just true would give anything in journalctl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added one liner log
| logger.error(f"failed to restart base.service. error: {errorMsg}") | ||
| raise Exception(errorMsg) | ||
|
|
||
| os.remove(f"{common.cloud_init_update_config_dir}/pim_config.json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you removing this before moving to the common.cloud_init_config_dir that's happening below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing it before otherwise pim_config.json will be packaged in cloud-init.iso
when we trigger launch after update-config.
pim_config.json is of no use in cloud-int.iso
| auth_data = json.loads(auth_json) | ||
| auth_json = json.dumps(auth_data, separators=(',', ':')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why this is required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auth json is in multiple line, which results in alignment breaking when we load auth.json in user-data template.
In this 2 line we are converting multi to a single line json
|
|
||
|
|
||
| def monitor_pim(config): | ||
| monitor_pim_boot(config) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we still need this, by checking the status of base.service, because if user don't want to validate means, there is nothing we are doing as part of monitor_pim method right? So we need some form of verification that upon bootstrap it booted into bootc image and at least started the base.service
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay will add it back
Signed-off-by: Adarsh Agrawal <[email protected]>
No description provided.