Skip to content

Log operator initial sync timings #329

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.

Sign up for GitHub

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

Merged
openshift-merge-robot merged 1 commit into from
Feb 2, 2019
Merged

Log operator initial sync timings #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions pkg/operator/operator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ const (
type Operator struct {
namespace, name string

inClusterBringup bool

imagesFile string

vStore *versionStore
Expand Down Expand Up @@ -165,6 +167,17 @@ func (optr *Operator) Run(workers int, stopCh <-chan struct{}) {
glog.Info("Starting MachineConfigOperator")
defer glog.Info("Shutting down MachineConfigOperator")

apiClient := optr.apiExtClient.ApiextensionsV1beta1()
_, err := apiClient.CustomResourceDefinitions().Get("machineconfigpools.machineconfiguration.openshift.io", metav1.GetOptions{})
if err != nil {
if apierrors.IsNotFound(err) {
glog.Infof("Couldn't find machineconfigpool CRD, in cluster bringup mode")
optr.inClusterBringup = true
} else {
glog.Errorf("While checking for cluster bringup: %v", err)
}
}

if !cache.WaitForCacheSync(stopCh,
optr.crdListerSynced,
optr.mcoconfigListerSynced,
Expand Down
28 changes: 19 additions & 9 deletions pkg/operator/sync.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,33 @@ import (
"github.com/openshift/machine-config-operator/pkg/version"
)

type syncFunc func(config renderConfig) error
type syncFunc struct {
name string
fn func(config renderConfig) error
}

func (optr *Operator) syncAll(rconfig renderConfig) error {
// syncFuncs is the list of sync functions that are executed in order.
// any error marks sync as failure but continues to next syncFunc
syncFuncs := []syncFunc{
optr.syncMachineConfigPools,
optr.syncMachineConfigController,
optr.syncMachineConfigServer,
optr.syncMachineConfigDaemon,
optr.syncRequiredMachineConfigPools,
var syncFuncs = []syncFunc{
{ "pools", optr.syncMachineConfigPools },
{ "mcc", optr.syncMachineConfigController },
{ "mcs", optr.syncMachineConfigServer },
{ "mcd", optr.syncMachineConfigDaemon },
{ "required-pools", optr.syncRequiredMachineConfigPools },
}

if err := optr.syncProgressingStatus(); err != nil {
return fmt.Errorf("error syncing progressing status: %v", err)
}

var errs []error
for _, f := range syncFuncs {
errs = append(errs, f(rconfig))
for _, sf := range syncFuncs {
startTime := time.Now()
errs = append(errs, sf.fn(rconfig))
if optr.inClusterBringup {
glog.Infof("[init mode] synced %s in %v", sf.name, time.Since(startTime))
Copy link
Member

@jlebon jlebon Jan 18, 2019

Choose a reason for hiding this comment

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

Oh this is neat!

Copy link
Contributor

@abhinavdahiya abhinavdahiya Jan 22, 2019

Choose a reason for hiding this comment

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

why report only on cluster bringup ?

Copy link
Member Author

@cgwalters cgwalters Jan 22, 2019

Choose a reason for hiding this comment

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

Currently there are some .V(4) level logs in sync(). It wasn't clear to me at what cadence we'd end up logging. Glancing at the operator...it watches all daemonsets in our namespace, so we'll do a sync every time e.g. a node updates and the MCD state changes right?

I guess we can do more sync logging if we more consistently add some rate-limiting/delays like #337 ?

}
optr.syncProgressingStatus()
}

Expand All @@ -48,6 +55,9 @@ func (optr *Operator) syncAll(rconfig renderConfig) error {
errs = append(errs, optr.syncFailingStatus(agg))
agg = utilerrors.NewAggregate(errs)
return fmt.Errorf("error syncing: %v", agg.Error())
} else if optr.inClusterBringup {
glog.Infof("Initialization complete")
optr.inClusterBringup = false
}

return optr.syncAvailableStatus()
Expand Down