Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

AGENT-59: Break assisted-service pod into separate systemd services #13

Merged
merged 8 commits into from Mar 31, 2022

Conversation

zaneb
Copy link
Member

@zaneb zaneb commented Mar 25, 2022

The podman kube play command has many limitations, especially when it comes to integration with systemd. Split the pod and its individual containers into separate systemd services instead.

Amongst other thing, this allows us to capture logs from all of these containers in the journal and associate them with separate services.

This script has not been adding anything useful since we started
templating the configmap to insert the host IP directly in
0024c85.
@zaneb zaneb changed the title Break assisted-service pod into separate systemd services AGENT-59: Break assisted-service pod into separate systemd services Mar 25, 2022
@@ -9,6 +10,7 @@ ExecStart=/usr/local/bin/create-cluster-and-infra-env.sh

KillMode=none
Type=oneshot
RemainAfterExit=true
Copy link
Collaborator

Choose a reason for hiding this comment

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

Being that is listed as PartOf, will that not trigger the creation if the assisted-service-pod restarts? Will this be prevented by RemainAfterExit? If not, we'll probably need to have the logic for checking if it is already done in create-cluster-and-infra-env.sh (which is not a bad idea)

Copy link
Member Author

Choose a reason for hiding this comment

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

Being PartOf has no real effect unless RemainAfterExit is also true (i.e. no restart is triggered by dependencies if a unit has already completed). Setting both ensures that this script gets re-run if the assisted-service pod restarts, which is necessary because a pod restart also clears the database.

@celebdor
Copy link
Collaborator

Looks good. Just a minor suggestion

Using "podman kube play" doesn't play that well with systemd, so split
the config up and run each container and the pod as a separate systemd
service.
The `podman ps` command just lists container names, not the pod they
belong to, so for clarity use less-generic names.
Create a separate environment file for each service that needs it.
Ensure that if the assisted-service pod is restarted, we will re-run the
create-cluster-and-infra-env script.
We don't actually need this for anything in the automated flow.
@@ -28,7 +28,7 @@ Run the tool using `go run cmd/main.go`.

The output ISO is written to `output/fleeting.iso`.

Boot the ISO in a VM with at least 4096MiB of RAM. No storage is required.
Boot the ISO in a VM with at least 8192MiB of RAM. No storage is required.
Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we are able to boot up the agents with ISO and we have this HW configuration https://github.com/openshift-agent-team/fleeting/pull/13/files#diff-514469c0981d360617b844887d7bed1da07f0dd12b8198001b83367b17319d4dR11, please update this information.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the whole README needs to be updated, but there's probably no point until we have a documented way for people to reproduce the environment.

IPV6_SUPPORT=true
NTP_DEFAULT_SERVER=
PUBLIC_CONTAINER_REGISTRIES=quay.io
RELEASE_IMAGES=[{"openshift_version":"4.10","cpu_architecture":"x86_64","url":"quay.io/openshift-release-dev/ocp-release:4.10.0-rc.1-x86_64","version":"4.10.0-rc.1"}]

Choose a reason for hiding this comment

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

Do we have any planned task to remove the hard-coded release image version?

Copy link
Member Author

Choose a reason for hiding this comment

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

It'll be part of integrating with the installer, since the installer knows what version it is.

Copy link
Contributor

@rwsu rwsu left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@pawanpinjarkar pawanpinjarkar left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci
Copy link

openshift-ci bot commented Mar 31, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pawanpinjarkar, zaneb

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

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [pawanpinjarkar,zaneb]

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

@openshift-merge-robot openshift-merge-robot merged commit e3a5a99 into openshift-agent-team:main Mar 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants