Skip to content
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

[wip] add systemd services for configuration after start #980

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

anjannath
Copy link
Member

@anjannath anjannath commented Oct 21, 2024

this adds 4 small systemd services that:

  • creates crc specific configurations for dnsmasq
  • sets a new uuid as cluster id
  • creates the pod for routes-controller
  • tries to grow the disk and filesystem
  • service that checks if all the cluster operators are ready

address=/apps-crc.testing/$ip
address=/api.crc.testing/$ip
address=/api-int.crc.testing/$ip
address=/$hostName.crc.testing/$iip
Copy link
Member

Choose a reason for hiding this comment

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

why we need this one for node name resolve (in OCP context) Also I think ip should be node name dummy IP which is 192.168.126.11?

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 probably not needed now, i think this was needed before moving to SNO and in the crc code-base we still have it so added here as well

Copy link
Member

Choose a reason for hiding this comment

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

did we test it with dummy IP 192.168.126.11 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

you mean using address=/$hostName.crc.testing/192.168.126.11? instead of use the result from hostname -i?

Copy link
Member

Choose a reason for hiding this comment

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

yes right, because this is used by internal service resolution so it might work.

systemd/crc-dnsmasq.sh Outdated Show resolved Hide resolved
createdisk.sh Outdated Show resolved Hide resolved
systemd/crc-dnsmasq.sh Outdated Show resolved Hide resolved
@anjannath anjannath force-pushed the issue_638 branch 4 times, most recently from 8ccdb6e to bd2b783 Compare October 24, 2024 06:13
@anjannath anjannath force-pushed the issue_638 branch 5 times, most recently from 525b66d to 1f5383b Compare November 18, 2024 08:37
done

# need to set a marker to let `crc` know the cluster is ready
# touch /tmp/.crc-cluster-ready
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating a file in the guest, you can also write a 'ready' notification message to a socket. podman does something similar: cfergeau@9e80530

[ $retry == $max_retry ] && exit 1
sleep 5
((retry++))
done
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if it is common/recommended to do this, but it might be possible to express "oc get secret ready" as a systemd unit

@anjannath
Copy link
Member Author

/test e2e-snc

@anjannath
Copy link
Member Author

/retest

@anjannath anjannath force-pushed the issue_638 branch 2 times, most recently from 9fd0545 to e6a4516 Compare November 27, 2024 09:13

if [ -z $CRC_CLOUD ]; then
exit 1
fi
Copy link
Member

Choose a reason for hiding this comment

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

Copy link

openshift-ci bot commented Dec 16, 2024

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please ask for approval from anjannath. For more information see the Code Review Process.

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@anjannath
Copy link
Member Author

/retest

2 similar comments
@anjannath
Copy link
Member Author

/retest

@anjannath
Copy link
Member Author

/retest

@@ -0,0 +1,24 @@
# Self sufficient bundles

Since release 4.19.0 of OpenShift Local, the bundles generated by `snc` contains additional systemd services to provision the cluster and removes the need of

Choose a reason for hiding this comment

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

Suggested change
Since release 4.19.0 of OpenShift Local, the bundles generated by `snc` contains additional systemd services to provision the cluster and removes the need of
Since release 4.19.0 of OpenShift Local, the bundles generated by `snc` contain additional systemd services to provision the cluster and remove the need for

# Self sufficient bundles

Since release 4.19.0 of OpenShift Local, the bundles generated by `snc` contains additional systemd services to provision the cluster and removes the need of
an outside entity for provision the cluster, although an outside process needs to create some files on pre-defined locations inside the VM for the systemd

Choose a reason for hiding this comment

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

Suggested change
an outside entity for provision the cluster, although an outside process needs to create some files on pre-defined locations inside the VM for the systemd
an outside entity to provision the cluster, although an outside process needs to create some files on pre-defined locations inside the VM for the systemd

an outside entity for provision the cluster, although an outside process needs to create some files on pre-defined locations inside the VM for the systemd
services to do their work.

## The following table lists the systemd services and the location of files they need to provision the cluster, users of SNC needs to create those files

Choose a reason for hiding this comment

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

Suggested change
## The following table lists the systemd services and the location of files they need to provision the cluster, users of SNC needs to create those files
## The following table lists the systemd services and the location of files they need to provision the cluster, users of SNC need to create those files


> [!NOTE]
> "Marker env variable" is set using an env file, if the required env variable is not set then unit is skipped
> some units are ran only when CRC_CLOUD=1 is set, these are only needed when using the bundles with crc-cloud

Choose a reason for hiding this comment

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

Suggested change
> some units are ran only when CRC_CLOUD=1 is set, these are only needed when using the bundles with crc-cloud
> some units are run only when CRC_CLOUD=1 is set, these are only needed when using the bundles with crc-cloud

done

# need to set a marker to let `crc` know the cluster is ready
# touch /tmp/.crc-cluster-ready

Choose a reason for hiding this comment

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

nit, Are these commented-out lines important? If not, shall we remove them?

return 1
}

# rm -rf /tmp/.crc-cluster-ready

Choose a reason for hiding this comment

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

Comment on lines 8 to 15
retry=0
max_retry=20
until `oc get clusterversion > /dev/null 2>&1`
do
[ $retry == $max_retry ] && exit 1
sleep 5
((retry++))
done

Choose a reason for hiding this comment

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

I see this block of code duplicated across various scripts where we are waiting for some resource to become available. I'm not 100% sure if it's possible to do it in SystemD setup but can we move this to a generic method that can handle various resource types? WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe with https://fedoramagazine.org/systemd-template-unit-files/
Or just by using a shared script.

Copy link
Member Author

Choose a reason for hiding this comment

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

sure, I'll extract this into a function that we can re-use across scripts

@anjannath anjannath force-pushed the issue_638 branch 3 times, most recently from 12df080 to c4d5572 Compare December 19, 2024 11:06
echo "${existingPs}" | jq -e '.auths'

if [[ $? != 0 ]]; then
pullSecretB64=$(cat /opt/crc/pull-secret)
Copy link
Member

Choose a reason for hiding this comment

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

pullSecretB64=$(cat /opt/crc/pull-secret | base64 -w0)


export KUBECONFIG=/opt/kubeconfig

function check_cluster_unhealthy() {
Copy link
Member

Choose a reason for hiding this comment

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

name should be check_cluster_healthy and return value should be 0 in case successful and 1 in case failure.

CLUSTER_HEALTH_SLEEP=8
CLUSTER_HEALTH_RETRIES=500

while $(check_cluster_unhealthy)
Copy link
Member

Choose a reason for hiding this comment

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

this should change to while ! check_cluster_healthy

@anjannath anjannath force-pushed the issue_638 branch 3 times, most recently from a62f49b to 0ed44f4 Compare January 7, 2025 15:59
the services does the various needed tasks to setup
the ocp or microshift cluster, these systemd  units
runs small shell scripts which are based on:
https://github.com/crc-org/crc-cloud/blob/main/pkg/bundle/setup/clustersetup.sh

and does the following tasks:
- creates crc specific configurations for dnsmasq
- sets a new uuid as cluster id
- creates the pod for routes-controller
- tries to grow the disk and filesystem
- checks if the cluster operators are ready
- adds the pull secret to the cluster
- sets kubeadmin and developer user passwords
- sets a custom ca for authentication
- sets custom nip.io cluster domain
Copy link

openshift-ci bot commented Jan 8, 2025

@anjannath: 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-microshift 3881812 link true /test e2e-microshift
ci/prow/e2e-snc 3881812 link true /test e2e-snc

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants