Skip to content

AGENT-502: Enable agent tui #6898

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

Merged
openshift-merge-robot merged 3 commits into from
Mar 2, 2023
Merged

AGENT-502: Enable agent tui #6898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a61b4e
add agent-tui files in the iso
andfasano Feb 23, 2023
da88832
fix agent services order
andfasano Feb 23, 2023
2cc87af
bump up default version
andfasano Feb 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/openshift-install/testdata/agent/image/manifests/default_manifests.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ metadata:
name: openshift-was not built correctly
namespace: cluster0
spec:
releaseImage: registry.ci.openshift.org/origin/release:4.12
releaseImage: registry.ci.openshift.org/origin/release:4.13
status: {}
-- expected/infraenv.yaml --
metadata:
Expand Down
1 change: 1 addition & 0 deletions data/data/agent/files/etc/assisted/agent-installer.env.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,6 @@
# ASSISTED_SERVICE_HOST must be updated.
#
NODE_ZERO_IP={{.NodeZeroIP}}
LD_LIBRARY_PATH=/usr/local/bin/

@zaneb zaneb Feb 28, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is worth at least a comment.

RELEASE_IMAGE={{.ReleaseImage}}
AGENT_TUI_LOG_PATH=/var/log/agent/agent-tui.log
2 changes: 1 addition & 1 deletion data/data/agent/systemd/units/agent-interactive-console.service
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[Unit]
Description=Get interactive user configuration at boot
After=network-pre.target NetworkManager.service pre-network-manager-config.service selinux.service
Before=network.target network.service agent.service NetworkManager-wait-online.service
Before=network.target network.service agent.service node-zero.service NetworkManager-wait-online.service

@zaneb zaneb Feb 28, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm surprised getty@tty1.service is not on this list

@zaneb zaneb Mar 1, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is network.service a real thing?

I thought for a minute that we should use Before=getty-pre.target (https://www.freedesktop.org/software/systemd/man/systemd.special.html#getty-pre.target), but that will block all consoles and we are only displaying on one console, so I think Before=getty@tty1.service is the right thing.


[Service]
Type=oneshot
Expand Down
2 changes: 1 addition & 1 deletion data/data/agent/systemd/units/set-hostname.service
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
Description=Agent-based installer hostname update service
Wants=network-online.target
After=local-fs.target
Before=node-zero.service
Before=agent-interactive-console.service

@zaneb zaneb Feb 28, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wouldn't hurt to list both here.


[Service]
ExecStart=/usr/local/bin/set-hostname.sh
Expand Down
42 changes: 23 additions & 19 deletions pkg/asset/agent/image/agentimage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,12 @@ func (a *AgentImage) Generate(dependencies asset.Parents) error {
return err
}

additionalFiles := []string{}
agentTuiFile, err := a.fetchAgentTuiBinary(agentManifests.ClusterImageSet.Spec.ReleaseImage, agentManifests.GetPullSecretData(), registriesConf.MirrorConfig)
agentTuiFiles, err := a.fetchAgentTuiFiles(agentManifests.ClusterImageSet.Spec.ReleaseImage, agentManifests.GetPullSecretData(), registriesConf.MirrorConfig)
if err != nil {
return err
}
additionalFiles = append(additionalFiles, agentTuiFile)

err = a.prepareAgentISO(baseImage.File.Filename, ignitionByte, additionalFiles)
err = a.prepareAgentISO(baseImage.File.Filename, ignitionByte, agentTuiFiles)
if err != nil {
return err
}
Expand All @@ -71,25 +69,28 @@ func (a *AgentImage) Generate(dependencies asset.Parents) error {
return nil
}

func (a *AgentImage) fetchAgentTuiBinary(releaseImage string, pullSecret string, mirrorConfig []mirror.RegistriesConfig) (string, error) {
r := NewRelease(&executer.CommonExecuter{},
func (a *AgentImage) fetchAgentTuiFiles(releaseImage string, pullSecret string, mirrorConfig []mirror.RegistriesConfig) ([]string, error) {
release := NewRelease(&executer.CommonExecuter{},
Config{MaxTries: OcDefaultTries, RetryDelay: OcDefaultRetryDelay},
releaseImage, pullSecret, mirrorConfig)

// TODO: This is just a temporary placeholder to test the entire workflow.
// As soon as https://github.com/openshift/assisted-installer-agent/pull/482 will land, then
// the following line should be fixed to extract the agent-tui binary
filename, err := r.ExtractFile("agent-installer-node-agent", "/usr/bin/agent")
if err != nil {
return "", err
}
// Make sure it could be executed
err = os.Chmod(filename, 0o555)
if err != nil {
return "", err
agentTuiFilenames := []string{"/usr/bin/agent-tui", "/usr/lib64/libnmstate.so.1.3.3"}
files := []string{}

for _, srcFile := range agentTuiFilenames {
f, err := release.ExtractFile("agent-installer-node-agent", srcFile)
if err != nil {
return nil, err
}
// Make sure it could be executed
err = os.Chmod(f, 0o555)
if err != nil {
return nil, err
}
files = append(files, f)
}

return filename, nil
return files, nil
}

func (a *AgentImage) prepareAgentISO(iso string, ignition []byte, additionalFiles []string) error {
Expand Down Expand Up @@ -181,7 +182,10 @@ func (a *AgentImage) appendAgentFilesToInitrd(additionalFiles []string) error {
// Add a dracut hook to copy the files. The $NEWROOT environment variable is exported by
// dracut during the startup and it refers the mountpoint for the root filesystem.
dracutHookScript := `#!/bin/sh
cp -R /agent-files/* $NEWROOT/usr/local/bin/`
cp -R /agent-files/* $NEWROOT/usr/local/bin/

@zaneb zaneb Feb 28, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In a perfect world, *.so* files would end up being copied to /usr/local/lib64.
Maybe we should put the a path hierarchy under agent-files? (i.e. store stuff as /agent-files/bin/agent-tui and do rsync -a /agent-files/ /usr/local)

@andfasano andfasano Feb 28, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In general I agree, in the sense that this is just a first rough mechanism to copy over stuff from the initramfs to the disk to have an initial integration, and it could be definitely improved as our requirements increase. We could follouwup in a subsequent pr

# Fix the selinux label
for i in $(find /agent-files/ -printf "%P\n"); do chcon system_u:object_r:bin_t:s0 $NEWROOT/usr/local/bin/$i; done`

@zaneb zaneb Feb 28, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If restorecon -F worked and avoided having to explicitly specify the type, that seems preferable.

@andfasano andfasano Feb 28, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure if I've missed something, but in my tests restorecon wasn't available

@zaneb zaneb Mar 1, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It's in CoreOS, but maybe not in the initramfs 🙁


err = ca.StoreBytes("/usr/lib/dracut/hooks/pre-pivot/99-agent-copy-files.sh", []byte(dracutHookScript), 0o755)
if err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion pkg/asset/releaseimage/default.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (

var (
// defaultReleaseImageOriginal is the value served when defaultReleaseImagePadded is unmodified.
defaultReleaseImageOriginal = "registry.ci.openshift.org/origin/release:4.12"
defaultReleaseImageOriginal = "registry.ci.openshift.org/origin/release:4.13"
Comment thread
andfasano marked this conversation as resolved.
Outdated
// defaultReleaseImagePadded may be replaced in the binary with a pull spec that overrides defaultReleaseImage as
// a null-terminated string within the allowed character length. This allows a distributor to override the payload
// location without having to rebuild the source.
Expand Down