diff --git a/pkg/operator/operator.go b/pkg/operator/operator.go index ea58191b1d..6b313528ad 100644 --- a/pkg/operator/operator.go +++ b/pkg/operator/operator.go @@ -4,9 +4,12 @@ import ( "fmt" "time" + "github.com/openshift/machine-api-operator/pkg/version" + "github.com/golang/glog" osclientset "github.com/openshift/client-go/config/clientset/versioned" + osconfigv1 "github.com/openshift/api/config/v1" v1 "k8s.io/api/core/v1" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/wait" @@ -50,7 +53,8 @@ type Operator struct { deployListerSynced cache.InformerSynced // queue only ever has one item, but it has nice error handling backoff/retry semantics - queue workqueue.RateLimitingInterface + queue workqueue.RateLimitingInterface + operandVersions []osconfigv1.OperandVersion } // New returns a new machine config operator. @@ -80,6 +84,12 @@ func New( osClient: osClient, eventRecorder: eventBroadcaster.NewRecorder(scheme.Scheme, v1.EventSource{Component: "machineapioperator"}), queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "machineapioperator"), + operandVersions: []osconfigv1.OperandVersion{ + { + Name: "operator", + Version: version.Raw, + }, + }, } serviceAccountInfomer.Informer().AddEventHandler(optr.eventHandler()) diff --git a/pkg/operator/status.go b/pkg/operator/status.go index f15ce98388..249781ad98 100644 --- a/pkg/operator/status.go +++ b/pkg/operator/status.go @@ -1,13 +1,16 @@ package operator import ( + "fmt" + "reflect" + "strings" + + "github.com/golang/glog" + osconfigv1 "github.com/openshift/api/config/v1" - osclientsetv1 "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1" cvoresourcemerge "github.com/openshift/cluster-version-operator/lib/resourcemerge" - "github.com/openshift/machine-api-operator/pkg/version" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/utils/pointer" ) // StatusReason is a MixedCaps string representing the reason for a @@ -28,17 +31,31 @@ const ( // statusProgressing sets the Progressing condition to True, with the given // reason and message, and sets both the Available and Failing conditions to // False. -func (optr *Operator) statusProgressing(reason StatusReason, message string) error { +func (optr *Operator) statusProgressing() error { + desiredVersions := optr.operandVersions + currentVersions, err := optr.getCurrentVersions() + if err != nil { + glog.Errorf("error getting current versions: %v", err) + return err + } + + var message string + if !reflect.DeepEqual(desiredVersions, currentVersions) { + message = fmt.Sprintf("Progressing towards %s", optr.printOperandVersions()) + } else { + message = fmt.Sprintf("Running resync for %s", optr.printOperandVersions()) + } + conds := []osconfigv1.ClusterOperatorStatusCondition{ { Type: osconfigv1.OperatorProgressing, Status: osconfigv1.ConditionTrue, - Reason: string(reason), + Reason: string(ReasonSyncing), Message: message, }, { Type: osconfigv1.OperatorAvailable, - Status: osconfigv1.ConditionFalse, + Status: osconfigv1.ConditionTrue, }, { Type: osconfigv1.OperatorFailing, @@ -46,18 +63,23 @@ func (optr *Operator) statusProgressing(reason StatusReason, message string) err }, } - return optr.syncStatus(conds) + co, err := optr.getOrCreateClusterOperator() + if err != nil { + glog.Errorf("failed to get or create Cluster Operator: %v", err) + return err + } + return optr.syncStatus(co, conds) } // statusAvailable sets the Available condition to True, with the given reason // and message, and sets both the Progressing and Failing conditions to False. -func (optr *Operator) statusAvailable(reason StatusReason, message string) error { +func (optr *Operator) statusAvailable() error { conds := []osconfigv1.ClusterOperatorStatusCondition{ { Type: osconfigv1.OperatorAvailable, Status: osconfigv1.ConditionTrue, - Reason: string(reason), - Message: message, + Reason: string(ReasonEmpty), + Message: fmt.Sprintf("Cluster Machine API Operator is available at %s", optr.printOperandVersions()), }, { Type: osconfigv1.OperatorProgressing, @@ -70,19 +92,38 @@ func (optr *Operator) statusAvailable(reason StatusReason, message string) error }, } - return optr.syncStatus(conds) + co, err := optr.getOrCreateClusterOperator() + if err != nil { + return err + } + co.Status.Versions = optr.operandVersions + return optr.syncStatus(co, conds) } // statusFailing sets the Failing condition to True, with the given reason and // message, and sets the Progressing condition to False, and the Available // condition to True. This indicates that the operator is present and may be // partially functioning, but is in a degraded or failing state. -func (optr *Operator) statusFailing(reason StatusReason, message string) error { +func (optr *Operator) statusFailing(error string) error { + desiredVersions := optr.operandVersions + currentVersions, err := optr.getCurrentVersions() + if err != nil { + glog.Errorf("error getting current versions: %v", err) + return err + } + + var message string + if !reflect.DeepEqual(desiredVersions, currentVersions) { + message = fmt.Sprintf("Failed when progressing towards %s because %s", optr.printOperandVersions(), error) + } else { + message = fmt.Sprintf("Failed to resync for %s because %s", optr.printOperandVersions(), error) + } + conds := []osconfigv1.ClusterOperatorStatusCondition{ { Type: osconfigv1.OperatorFailing, Status: osconfigv1.ConditionTrue, - Reason: string(reason), + Reason: string(ReasonSyncFailed), Message: message, }, { @@ -95,60 +136,70 @@ func (optr *Operator) statusFailing(reason StatusReason, message string) error { }, } - return optr.syncStatus(conds) + co, err := optr.getOrCreateClusterOperator() + if err != nil { + return err + } + return optr.syncStatus(co, conds) } //syncStatus applies the new condition to the mao ClusterOperator object. -func (optr *Operator) syncStatus(conds []osconfigv1.ClusterOperatorStatusCondition) error { - // to report the status of all the managed components. - // TODO we will report the version of the operands (so our machine api implementation version) - // NOTE: related objects lets openshift/must-gather collect diagnostic content - clusterOperator := &osconfigv1.ClusterOperator{ - ObjectMeta: metav1.ObjectMeta{ - Name: clusterOperatorName, - }, - Status: osconfigv1.ClusterOperatorStatus{ - Versions: []osconfigv1.OperandVersion{ - { - Name: "machine-api", - Version: version.Raw, - }, - }, - RelatedObjects: []osconfigv1.ObjectReference{ - { - Group: "", - Resource: "namespaces", - Name: optr.namespace, - }, - }, - }, - } - +func (optr *Operator) syncStatus(co *osconfigv1.ClusterOperator, conds []osconfigv1.ClusterOperatorStatusCondition) error { for _, c := range conds { - cvoresourcemerge.SetOperatorStatusCondition(&clusterOperator.Status.Conditions, c) + cvoresourcemerge.SetOperatorStatusCondition(&co.Status.Conditions, c) } - _, _, err := ApplyClusterOperator(optr.osClient.ConfigV1(), clusterOperator) + _, err := optr.osClient.ConfigV1().ClusterOperators().UpdateStatus(co) return err } -// ApplyClusterOperator applies the required ClusterOperator -func ApplyClusterOperator(client osclientsetv1.ClusterOperatorsGetter, required *osconfigv1.ClusterOperator) (*osconfigv1.ClusterOperator, bool, error) { - existing, err := client.ClusterOperators().Get(required.Name, metav1.GetOptions{}) +func (optr *Operator) getOrCreateClusterOperator() (*osconfigv1.ClusterOperator, error) { + co, err := optr.osClient.ConfigV1().ClusterOperators().Get(clusterOperatorName, metav1.GetOptions{}) if errors.IsNotFound(err) { - actual, err := client.ClusterOperators().Create(required) - return actual, true, err + // to report the status of all the managed components. + // TODO we will report the version of the operands (so our machine api implementation version) + // NOTE: related objects lets openshift/must-gather collect diagnostic content + co = &osconfigv1.ClusterOperator{ + ObjectMeta: metav1.ObjectMeta{ + Name: clusterOperatorName, + }, + Status: osconfigv1.ClusterOperatorStatus{ + Versions: optr.operandVersions, + RelatedObjects: []osconfigv1.ObjectReference{ + { + Group: "", + Resource: "namespaces", + Name: optr.namespace, + }, + }, + }, + } + + glog.Infof("%s clusterOperator status does not exist, creating %v", clusterOperatorName, co) + co, err := optr.osClient.ConfigV1().ClusterOperators().Create(co) + if err != nil { + return nil, err + } + return co, nil } if err != nil { - return nil, false, err + return nil, err } + return co, nil +} - modified := pointer.BoolPtr(false) - cvoresourcemerge.EnsureClusterOperatorStatus(modified, existing, *required) - if !*modified { - return existing, false, nil +func (optr *Operator) getCurrentVersions() ([]osconfigv1.OperandVersion, error) { + co, err := optr.getOrCreateClusterOperator() + if err != nil { + return nil, err } + return co.Status.Versions, nil +} - actual, err := client.ClusterOperators().UpdateStatus(existing) - return actual, true, err +func (optr *Operator) printOperandVersions() string { + versionsOutput := []string{} + for _, operand := range optr.operandVersions { + versionsOutput = append(versionsOutput, fmt.Sprintf("%s: %s", operand.Name, operand.Version)) + } + return strings.Join(versionsOutput, ", ") } diff --git a/pkg/operator/status_test.go b/pkg/operator/status_test.go new file mode 100644 index 0000000000..85d4e4ae82 --- /dev/null +++ b/pkg/operator/status_test.go @@ -0,0 +1,27 @@ +package operator + +import ( + "testing" + + osconfigv1 "github.com/openshift/api/config/v1" +) + +func TestPrintOperandVersions(t *testing.T) { + optr := Operator{ + operandVersions: []osconfigv1.OperandVersion{ + { + Name: "operator", + Version: "1.0", + }, + { + Name: "controller-manager", + Version: "2.0", + }, + }, + } + expectedOutput := "operator: 1.0, controller-manager: 2.0" + got := optr.printOperandVersions() + if got != expectedOutput { + t.Errorf("Expected: %s, got: %s", expectedOutput, got) + } +} diff --git a/pkg/operator/sync.go b/pkg/operator/sync.go index 6075dda816..48cc025c45 100644 --- a/pkg/operator/sync.go +++ b/pkg/operator/sync.go @@ -24,13 +24,13 @@ const ( func (optr *Operator) syncAll(config OperatorConfig) error { glog.Infof("Syncing ClusterOperatorStatus") - if err := optr.statusProgressing(ReasonSyncing, "Running sync functions"); err != nil { + if err := optr.statusProgressing(); err != nil { glog.Errorf("Error synching ClusterOperatorStatus: %v", err) return fmt.Errorf("error syncing ClusterOperatorStatus: %v", err) } if err := optr.syncClusterAPIController(config); err != nil { - if err := optr.statusFailing(ReasonSyncFailed, err.Error()); err != nil { + if err := optr.statusFailing(err.Error()); err != nil { // Just log the error here. We still want to // return the outer error. glog.Errorf("Error synching ClusterOperatorStatus: %v", err) @@ -42,7 +42,7 @@ func (optr *Operator) syncAll(config OperatorConfig) error { glog.Info("Synched up cluster api controller") - if err := optr.statusAvailable(ReasonEmpty, "cluster-api ready"); err != nil { + if err := optr.statusAvailable(); err != nil { glog.Errorf("Error synching ClusterOperatorStatus: %v", err) return fmt.Errorf("error syncing ClusterOperatorStatus: %v", err) } diff --git a/test/integration/main.go b/test/integration/main.go index 4b9be491c7..0c44d2ee26 100644 --- a/test/integration/main.go +++ b/test/integration/main.go @@ -423,7 +423,8 @@ var rootCmd = &cobra.Command{ // TESTS // verify the cluster-api is running err = wait.Poll(pollInterval, timeoutPoolClusterAPIDeploymentInterval, func() (bool, error) { - if clusterAPIDeployment, err := testConfig.KubeClient.AppsV1beta2().Deployments(targetNamespace).Get("clusterapi-manager-controllers", metav1.GetOptions{}); err == nil { + clusterAPIDeployment, err := testConfig.KubeClient.AppsV1beta2().Deployments(targetNamespace).Get("clusterapi-manager-controllers", metav1.GetOptions{}) + if err == nil { // Check all the pods are running log.Infof("Waiting for all clusterapi-manager-controllers deployment pods to be ready, have %v, expecting 1", clusterAPIDeployment.Status.ReadyReplicas) if clusterAPIDeployment.Status.ReadyReplicas < 1 { @@ -432,7 +433,7 @@ var rootCmd = &cobra.Command{ return true, nil } - log.Info("Waiting for clusterapi-manager-controllers deployment to be created") + log.Infof("Waiting for clusterapi-manager-controllers deployment to be created: %v", err) return false, nil }) diff --git a/test/integration/manifests/status-crd.yaml b/test/integration/manifests/status-crd.yaml index ca4349fa0f..36f19268a2 100644 --- a/test/integration/manifests/status-crd.yaml +++ b/test/integration/manifests/status-crd.yaml @@ -30,8 +30,5 @@ spec: subresources: status: {} version: v1 - versions: - - name: v1 - served: true