add Precision Time Protocol support #4

zshi-redhat · 2019-09-03T10:34:14Z

No description provided.

oglok

Overall, it looks very good to me and very aligned to the prototype we've already done. Good job!

oglok · 2019-09-04T10:07:02Z

time/ptp.md

+#### PTP4L configuration file
+
+Default ptp4l configuration file (ptp4l.conf) will be used when starting ptp4l
+service, user doesn't need to specify ptp4l.conf in `ptp4lOpts`. The content


As I understand, the daemonset will use the default configuration from ptp4l.conf and the user can overwrite some of the options via ptp4lOpts. It would be worth to mention where this options will be stored (like a configMap or in /etc/sysconfig/ptp4l).

Correct!
updated the doc to add:

ptp4l.conf is installed in ptp daemon image by default under /etc/ptp4l.conf

-f ptp4l.conf will be automatically appended to ptp4lOpts by PTP daemon

some of ptp4l.conf configs can be overwritten by specifying certain options in ptp4lOpts.

oglok · 2019-09-04T10:11:48Z

time/ptp.md

+In case of failure in linuxptp configuration, OpenShift nodes will be left as
+no time source to sync, resulting in potential time disorder among nodes.
+This can be resolved by providing PTP and NTP redundancy via timemaster service
+which automatically rolls back to use default NTP time source if configured.


This looks like we could mitigate the risk of failure by using timemaster (which is right) but we don't support timemaster service configuration. Just to make it clear.

updated to explicitly mention :

timemaster service can mitigate the redundancy issue between PTP and NTP

timemaster configuration will not be supported in initial proposal.

oglok · 2019-09-04T10:14:11Z

time/ptp.md

+
+#### Tech Preview -> GA
+
+- Have an opeartor to manage PTP CRDs


Typo: operator

Eagle eyes! Thanks ! fixed

oglok · 2019-09-04T10:17:46Z

time/ptp.md

+## Infrastructure Needed [optional]
+
+This requires a github repo be created under openshift org to hold PTP
+DaemonSet code. The name of this repo is `ptp-daemon`.


As mentioned above, there will be the need of an operator managing the PTP daemonset. Then, there will be two different repos? one with the ptp-daemon and one with the ptp-operator?

Yes, the idea is to first implement a PTP daemon in 4.3 as Tech Preview, and have an operator to manage this daemon and exposes all CRDs mentioned in proposal. Operator will need a separate repo under openshift github org (this is not strictly required, as you can see, SR-IOV Operator uses one repo to hold code for both daemon and operator).

Yeah, that's why I mentioned it. I think the trend in OpenShift is to name the repos according to the operator, even if they contains the code of the actual application that is being "operated".

If the long term direction is to deliver this via an operator I think we should just go ahead and create the repo as if it's an operator.

After looking at this some more it seems like there are more examples of separate repos for operator and operands, ie: image-registry, kubernetes-apiserver. So it seems there's no norm established here so lets just go with separate repos as initially proposed.

time/ptp.md

sdodson · 2019-09-09T14:07:53Z

time/ptp.md

+## Infrastructure Needed [optional]
+
+This requires a github repo be created under openshift org to hold PTP
+DaemonSet code. The name of this repo is `ptp-daemon`.


If the long term direction is to deliver this via an operator I think we should just go ahead and create the repo as if it's an operator.

zshi-redhat · 2019-09-10T04:23:38Z

@sdodson thanks for the review!
Re the repo, I was thinking to create two repos eventually, one is for ptp daemon that manages linuxptp services, the other is for ptp operator which exposes CRDs. I'm not sure what's the best practice to create repos for such case. I have seen tuned operator and daemonset be separated in different repos, and SR-IOV config daemon and Operator located in the same repo. Do we have any guidance on whether to divide daemon and operator into different repos?

sdodson · 2019-09-11T16:46:54Z

@zshi-redhat lets go ahead with separate repos

zshi-redhat · 2019-09-16T06:26:47Z

@zshi-redhat lets go ahead with separate repos

@sdodson cool, thanks, will use separate repos for ptp-daemon and ptp-operator

fepan · 2019-09-30T14:20:28Z

@sdodson could you approve this PR, I think we are ready to merge.

sdodson · 2019-09-30T14:31:23Z

/approve

fepan · 2019-09-30T14:51:06Z

/lgtm

openshift-ci-robot · 2019-09-30T14:51:19Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fepan, sdodson, zshi-redhat

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~~ [sdodson]

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

ashcrow · 2019-11-21T18:22:59Z

I realize this already merged a while back and likely has had some refinement but I do want to ask a few questions:

How would one set up this operator? Is it enabled at install time via UPI? OLM?
Would it need to interact with any machine components/configurations (IE: interact with MCO)?
RHEL nodes are noted, RHCOS isn't. Is this meant for both?

zshi-redhat · 2019-11-21T22:49:46Z

I realize this already merged a while back and likely has had some refinement but I do want to ask a few questions:

How would one set up this operator? Is it enabled at install time via UPI? OLM?

OLM, it's an optional operator installed via openshift marketplace

Would it need to interact with any machine components/configurations (IE: interact with MCO)?

RHEL nodes are noted, RHCOS isn't. Is this meant for both?

Currently, no interaction with MCO. All ptp services are configured via this Opreator.
One thing we didn't add in operator, which do need to interact with MCO, is to disable chronyd service via MCO. Another thing I can think of is to use MCO for bonding interface creation and then PTP can be configured on top of bond to provide redundency and failover.
Both RHEL and RHCOS are supported.

ashcrow · 2019-11-22T14:35:36Z

OLM, it's an optional operator installed via openshift marketplace

Perfect

Currently, no interaction with MCO. All ptp services are configured via this Opreator.

Got it 👍

One thing we didn't add in operator, which do need to interact with MCO, is to disable chronyd service via MCO. Another thing I can think of is to use MCO for bonding interface creation and then PTP can be configured on top of bond to provide redundency and failover.

OK, for this please make sure to interact with the MCO team when you get to the point of designing this addition (cc'ing @runcom as an FYI as he leads that team).

Both RHEL and RHCOS are supported.

Awesome 😃

…ift#4)

Signed-off-by: Nir <niry@redhat.com>

ConfigMap: make it not exclusive for ipfix (it could be helpful in th…

os: un-RAID ESP; use GRUB RAID module

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 3, 2019

openshift-ci-robot requested review from chancez and mfojtik September 3, 2019 10:34

zshi-redhat force-pushed the ptp branch from cc7a822 to 2d628d3 Compare September 3, 2019 10:47

oglok reviewed Sep 4, 2019

View reviewed changes

zshi-redhat force-pushed the ptp branch from 2d628d3 to 38d7aa6 Compare September 5, 2019 08:29

add Precision Time Protocol support

7235896

zshi-redhat force-pushed the ptp branch from 38d7aa6 to 7235896 Compare September 9, 2019 11:14

sdodson reviewed Sep 9, 2019

View reviewed changes

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 30, 2019

openshift-ci-robot assigned fepan Sep 30, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 30, 2019

openshift-merge-robot merged commit 67fb6aa into openshift:master Sep 30, 2019

marun added a commit to marun/enhancements that referenced this pull request Jan 21, 2020

Fixup openshift#4

aa03965

marun added a commit to marun/enhancements that referenced this pull request Jan 21, 2020

Fixup openshift#4

c422477

syedriko mentioned this pull request Feb 26, 2020

A first stab at container log throttling enhancement proposal #231

Closed

pedjak added a commit to pedjak/openshift-enhancements that referenced this pull request Apr 15, 2020

added approvers and made endpoint design console backend free (opensh…

f68612f

…ift#4)

beekhof referenced this pull request in n1r1/enhancements Nov 20, 2020

add poison pill enhancement

290ef08

Signed-off-by: Nir <niry@redhat.com>

sferich888 mentioned this pull request Dec 18, 2020

baremetal: Propose BMC-less remediation enhancement (AKA poison pill) #547

Closed

openshift-merge-robot pushed a commit that referenced this pull request Oct 18, 2021

Merge pull request #4 from jotak/cm-ipfix-non-exclusive

fd2a4b7

ConfigMap: make it not exclusive for ipfix (it could be helpful in th…

cgwalters pushed a commit to cgwalters/enhancements that referenced this pull request Oct 19, 2021

Merge pull request openshift#4 from bgilbert/raid

573564d

os: un-RAID ESP; use GRUB RAID module

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add Precision Time Protocol support #4

add Precision Time Protocol support #4

zshi-redhat commented Sep 3, 2019

oglok left a comment

oglok Sep 4, 2019

zshi-redhat Sep 5, 2019

oglok Sep 5, 2019

oglok Sep 4, 2019

zshi-redhat Sep 5, 2019

oglok Sep 5, 2019

oglok Sep 4, 2019

zshi-redhat Sep 5, 2019

oglok Sep 4, 2019

zshi-redhat Sep 5, 2019

oglok Sep 5, 2019

sdodson Sep 9, 2019

sdodson Sep 11, 2019

sdodson Sep 9, 2019

zshi-redhat commented Sep 10, 2019

sdodson commented Sep 11, 2019

zshi-redhat commented Sep 16, 2019

fepan commented Sep 30, 2019

sdodson commented Sep 30, 2019

fepan commented Sep 30, 2019

openshift-ci-robot commented Sep 30, 2019

ashcrow commented Nov 21, 2019

zshi-redhat commented Nov 21, 2019 •

edited

Loading

ashcrow commented Nov 22, 2019


		#### Tech Preview -> GA

		- Have an opeartor to manage PTP CRDs

add Precision Time Protocol support #4

add Precision Time Protocol support #4

Conversation

zshi-redhat commented Sep 3, 2019

oglok left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zshi-redhat commented Sep 10, 2019

sdodson commented Sep 11, 2019

zshi-redhat commented Sep 16, 2019

fepan commented Sep 30, 2019

sdodson commented Sep 30, 2019

fepan commented Sep 30, 2019

openshift-ci-robot commented Sep 30, 2019

ashcrow commented Nov 21, 2019

zshi-redhat commented Nov 21, 2019 • edited Loading

ashcrow commented Nov 22, 2019

zshi-redhat commented Nov 21, 2019 •

edited

Loading