Skip to content

Conversation

@rwsu
Copy link
Contributor

@rwsu rwsu commented Jan 6, 2023

The node zero IP and ServiceBaseURL are hard-coded into scripts and
systemd service definition files. This makes it difficult to change
the node zero IP once the agent ISO has booted up. To facilitate
changing these settings in the interactive network configuration
use case, the hard-codings have been replaced with sourcing files
containing environment variables.

In the interactive network configuration scenario, the environment
files will be updated by the network configuration utility. The
agent services are blocked by the interactive console service. When
the interactive console service finishes, the agent services startup
and read in the environment files picking up any changes.

In the non-interactive scenario, the environment files will be
pre-populated with values for node zero IP and SERVICE_BASE_URL in
the Agent ISO. This is how it works today.

rwsu added 2 commits January 6, 2023 04:35
The node zero IP and ServiceBaseURL are hard-coded into scripts and
systemd service definition files. This makes it difficult to change
the node zero IP once the agent ISO has booted up. To faciliate
changing these settings in the interactive network configuration
use case, the hard-codings have been replaced with sourcing files
containing environment variables.

In the interactive network configuration scenario, the environment
files will be updated by the network configuration utility. The
agent services are blocked by the interactive console service. When
the interactive console service finishes, the agent services start
up and read in the environment files picking up any changes.

In the non-interactive scenario, the environment files will be
prepopulated with values for node zero IP and SERVICE_BASE_URL in
the Agent ISO. This is how it works today.

Signed-off-by: Richard Su <[email protected]>
The agent-installer manifests and host configuration files are
already in /etc/assisted/. Having a node0 file under
/etc/assisted-service/ feels out of place because it is the only
file in that sub-directory.

Added documentation explaining how /etc/assisted/node0 is used.
# definitions:
# apply-host-config.service
# assisted-service-pod.service
# assisted-service.service
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit - It doesn't look like there is a ConditionPathExists in assisted-service.service. Will need to either add it or change this comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Other than that, looks good. I have not tested it yet but I will.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch.

@andfasano
Copy link
Contributor

/retest-required

@andfasano
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 12, 2023
@bfournie
Copy link
Contributor

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 12, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bfournie

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 12, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 12, 2023

@rwsu: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-ovn-disruptive c4b507e link false /test e2e-aws-ovn-disruptive
ci/prow/e2e-agent-sno-ipv6 c4b507e link false /test e2e-agent-sno-ipv6

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-ci-robot
Copy link
Contributor

/retest-required

Remaining retests: 0 against base HEAD 8c83507 and 2 for PR HEAD c4b507e in total

@openshift-merge-robot openshift-merge-robot merged commit 9f1c1dc into openshift:master Jan 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants