Skip to content

Conversation

@aneeshkp
Copy link
Collaborator

@aneeshkp aneeshkp commented Oct 3, 2025

PTP: add PtpConfig status conditions API, helper, and RBAC for operand updates
Introduces status.ptpStatus.conditions[] on PtpConfig to expose runtime health/state per profile and active config file.
Adds a small helper API for operands (linuxptp-daemon, cloud-event-proxy) to upsert conditions via the status subresource.
Wires RBAC so linuxptp-daemon can update PtpConfig status.
Documents the schema and examples in README.

cond := ptpv1.PtpCondition{
Type: ptpv1.ConditionPTP4lRunning,
Profile: "GM-Profile-A",
Filename: "ptp4l.0.config",
Status: corev1.ConditionTrue,
Reason: "Started",
Message: "ptp4l is running",
LastUpdateTime: metav1.Now(),
}
_ = ptpstatus.UpsertPtpCondition(ctx, cs, "openshift-ptp", "default", cond)

Te operand can import the API call call this to update (RBAC are updated for linuxptp-daemon pods)

import (
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/rest"

ptpv1 "github.com/k8snetworkplumbingwg/ptp-operator/api/v1"
clientset "github.com/k8snetworkplumbingwg/ptp-operator/pkg/client/clientset/versioned"
ptpstatus "github.com/k8snetworkplumbingwg/ptp-operator/pkg/status"
)

cfg, _ := rest.InClusterConfig()
cs, _ := clientset.NewForConfig(cfg)
_ = ptpstatus.UpsertPtpCondition(ctx, cs, "openshift-ptp", "default", ptpv1.PtpCondition{
Type: ptpv1.ConditionPTP4lRunning,
Profile: "GM-Profile-A",
Filename: "ptp4l.0.config",
Status: corev1.ConditionTrue,
Reason: "Started",
Message: "ptp4l is running",
LastUpdateTime: metav1.Now(),
})

}

// enforce max history
const maxConditions = 100
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we just want to do N conditions or should One of the same type replace it e.g if PTP4L is no longer running I would expect the running condition to be removed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm, I think we thought of replacing same kind , but same kind means we need to match with conditionType and other paramters file profile name and file name , to support multiple profiles

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if c.Type == cond.Type && c.Profile == cond.Profile && c.Filename == cond.Filename && c.Process == cond.Process {
*c = cond
updated = true
break
}

// enforce max history
const maxConditions = 100
if n := len(pc.Status.PtpStatus.Conditions); n > maxConditions {
pc.Status.PtpStatus.Conditions = pc.Status.PtpStatus.Conditions[n-maxConditions:]
Copy link
Collaborator

Choose a reason for hiding this comment

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

So if one process (e.g. ptp4l) has too many statuses, we won't see detail from other processes such as ts2phc if it didn't report more recently than the last 100 ptp4l?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point , i should filter them by processname , will add that process name + profile name + file name (ptp4l.0.config )

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed , now upsert by composite key (type, profile, filename, process), so repeated ptp4l updates overwrite the same entry instead of appending. They won’t evict ts2phc, etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

now only if we have 100 distinct keys then we evict

@aneeshkp aneeshkp force-pushed the add-status-ptpconfig branch from 40a9ab8 to b80b607 Compare October 3, 2025 16:14
@aneeshkp aneeshkp force-pushed the add-status-ptpconfig branch from b80b607 to 345311e Compare October 3, 2025 16:25
nocturnalastro
nocturnalastro previously approved these changes Oct 3, 2025
Copy link
Collaborator

@nocturnalastro nocturnalastro left a comment

Choose a reason for hiding this comment

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

/lgtm

ConditionPhc2sysRunning PtpConditionType = "Phc2sysRunning"
ConditionTs2phcRunning PtpConditionType = "Ts2phcRunning"
ConditionSynce4lRunning PtpConditionType = "Synce4lRunning"
ConditionChronydRunning PtpConditionType = "ChronydRunning"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should gpspipe and gpsd be added here?

Signed-off-by: Aneesh Puttur <[email protected]>
@aneeshkp aneeshkp force-pushed the add-status-ptpconfig branch from 4a48ac5 to df5a320 Compare October 3, 2025 17:06
@josephdrichard
Copy link
Collaborator

What would this look like on daemon side?

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.

3 participants