-
Notifications
You must be signed in to change notification settings - Fork 103
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
https://issues.redhat.com/browse/ACM-17505--Add Manage and Configure OS to Edge Manager #7531
base: 2.13_stage
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: amolnar-rh The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
d47db6b
to
7b4db99
Compare
9436860
to
06613d4
Compare
[#manage-os-config] | ||
= Operating system configuration | ||
|
||
You can include an operating system-level host configuration in the image to provide maximum consistency and repeatability. |
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.
When saying "repeatability", does it refer to deploying/updating with the same image on multiple devices?
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.
Yes - ensuring all devices have that same configuration
[source,bash] | ||
---- | ||
flightctl apply -f some_device_name.yaml | ||
---- |
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.
Can we add verification or monitoring?
06613d4
to
6b019e2
Compare
…OS to Edge Manager
6b019e2
to
036ca76
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.
Overall looks great, just a few comments
site-settings git https://github.com/<your_org>/<your_repo>.git True | ||
---- | ||
|
||
. To apply the `example-site` configuration to a device, update the device specification as follows: |
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 would clarify three things:
- Explicitly explain that this configuration will take the files and directories in the directory "example-site" of the git repository, at the branch or tag "production", and put those files and directories at "/" on the device.
- "/" is not writeable in bootc systems - we expect the user to create a directory structure in git to describe where to place the files.
- RHEM does not monitor the git repository branch/tag for changes. Pushing updates to git will not cause RHEM to update the configuration. RHEM will only update devices when the Device specification changes. Therefore we recommend using version tags for targetRevision. Git hashes can also be used.
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 have two questions:
"/" is not writeable in bootc systems - we expect the user to create a directory structure in git to describe where to place the files.
- I can highlight it but in this case, it'd be clearer to provide a valid example. Could we use
/etc/example-site
instead? If yes, this could nicely tie in with the image building best practices (site-specific config is recommended in/etc
).
RHEM does not monitor the git repository branch/tag for changes. Pushing updates to git will not cause RHEM to update the configuration. RHEM will only update devices when the Device specification changes. Therefore we recommend using version tags for targetRevision. Git hashes can also be used.
- How do you sync your device spec from a Git repo? Use git for version control and then you manually update the device spec? Btw, the phrasing here implies that it automatically syncs the config once you set up a repo with RHEM.
[#manage-os-config] | ||
= Operating system configuration | ||
|
||
You can include an operating system-level host configuration in the image to provide maximum consistency and repeatability. |
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.
Yes - ensuring all devices have that same configuration
|
||
|`Path`|The subdirectory of the repository that contains the configuration. | ||
|
||
|`MountPath`|Optional. The directory in the file system of the device to which the configuration is written. By default, the value is the file system root `/`. |
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.
@fzdarsky Are we removing this parameter? If so, we should do it now
@@ -0,0 +1,25 @@ | |||
[#manage-devices] |
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.
Duplicate with edge_manager/edge_mgr_manage_devices_intro.adoc?
Anyway, see my comments in #7490, please.
* The configuration needs to be specific to a device. | ||
* The configuration needs to be updateable at runtime without updating the operating system image and rebooting. | ||
|
||
For these cases, you can declare a set of configuration files that can be present on the file system of the device. |
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.
For these cases, you can declare a set of configuration files that can be present on the file system of the device. | |
For these cases, you can declare a set of configuration files that shall be present on the file system of the device. |
You're declaring a desired state, not a possibility.
The {rhem} agent applies updates to the configuration files while ensuring that either all files are successfully updated in the file system, or rolled back to their pre-update state. | ||
If the user updates both an operating system and configuration set of a device at the same time, the {rhem} agent updates the operating system first, then applies the specified set of configuration files. | ||
|
||
You can also specify a list of configurations sets that the {rhem} agent applies in sequence. |
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.
You can also specify a list of configurations sets that the {rhem} agent applies in sequence. | |
You can also specify a list of configuration sets that the {rhem} agent applies in sequence. |
*Important:* After the {rhem} agent updates the configuration on the disk, the configuration must be activated. | ||
Running applications need to reload the new configuration into memory for the configuration to become effective. |
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 it clear that "activation" = "reloading the new configuration into memory, such that it becomes effective"? Otherwise, maybe a colon would help?
*Important:* After the {rhem} agent updates the configuration on the disk, the configuration must be activated. | |
Running applications need to reload the new configuration into memory for the configuration to become effective. | |
*Important:* After the {rhem} agent updates the configuration on the disk, the configuration must be activated: Running applications need to reload the new configuration into memory for the configuration to become effective. |
You can store device configuration in a Git repository such as GitHub or GitLab. | ||
The {rhem} synchronizes the configuration from the repository to the file system of the device by adding a Git Config Provider. |
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.
"by adding a Git Config Provider" seems to refer to "The {rhem} synchronizes" here, while it should refer to the "You can". Maybe:
You can store device configuration in a Git repository such as GitHub or GitLab. | |
The {rhem} synchronizes the configuration from the repository to the file system of the device by adding a Git Config Provider. | |
You can store device configuration in a Git repository such as GitHub or GitLab. You can then add a Git Config Provider, so that the {rhem} synchronizes the configuration from the repository to the file system of the device. |
Issue: https://issues.redhat.com/browse/ACM-17505
Preview: https://drive.google.com/drive/folders/1G984Za5pWqW_EW6qkaPFUoeeJTY22PlJ (click on file named after the branch)