openshift-kni / cnf-features-deploy

Kustomize configs for installing CNF features and e2e functional tests for verifying feature deployment/integration
Apache License 2.0
58 stars 135 forks source link

Chronyd and PTP should cannot be enabled in the same node #522

Open williamcaban opened 3 years ago

williamcaban commented 3 years ago

When using PTP Operator, it does contain the phy2sys which does time sync between PTP and the system. Having chronyd also active and doing clock sync, will be a conflict of two different processes (one with high time resolution other with regular time resolution) modifying the system clock. This affect workloads that rely on PTP and assume the system is also sync to PTP.

Remediation:

ijolliffe commented 3 years ago

@josephdrichard please have a look.

yuvalk commented 3 years ago

like this https://github.com/openshift-kni/cnf-features-deploy/blob/master/feature-configs/5g-ref-nw/profile-base/du-general/ptp/disable-chronyd/disable-chronyd.yaml ?

I'd say though, we do miss an automated test for it.

williamcaban commented 3 years ago

Yes, like that and to make sure it is part of the ZTP policy generator

vitus133 commented 3 years ago

@serngawy @imiller0 Could you guys please add it to the pipeline manifests? The need for patch is going to stay here for a while. As I understand it's not going to be handled by the PTP operator because there is a case where PTP benefits from NTP,

serngawy commented 3 years ago

@vitus133 yes, we have it https://github.com/openshift-kni/cnf-features-deploy/blob/master/ztp/source-policy-crs/MachineConfigDisableChronyd.yaml

I believe @imiller0 included it in the pipeline for SNO testing

imiller0 commented 3 years ago

Yes. This machine config CR is part of the DU profile that is applied to clusters deployed through our ZTP pipeline.

imiller0 commented 3 years ago

Yesterday I added a validation in the ZTP metrics pipeline which checks that the chronyd machine config (disables chronyd) is applied to the active SNO MCP.

ijolliffe commented 3 years ago

Thanks all and @imiller0 for the updates - seems like we can close this issue.