Skip to content

Commit

Permalink
make the status controller use SSA
Browse files Browse the repository at this point in the history
  • Loading branch information
deads2k committed Oct 22, 2024
1 parent 4c1f8b4 commit c774bfa
Show file tree
Hide file tree
Showing 9 changed files with 588 additions and 294 deletions.
99 changes: 38 additions & 61 deletions pkg/config/clusteroperator/v1helpers/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,57 +3,20 @@ package v1helpers
import (
"bytes"
"fmt"
"strings"
"time"

applyconfigv1 "github.com/openshift/client-go/config/applyconfigurations/config/v1"
"k8s.io/apimachinery/pkg/api/equality"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/util/diff"
"k8s.io/apimachinery/pkg/util/json"
"k8s.io/utils/ptr"
"strings"

configv1 "github.com/openshift/api/config/v1"
)

// SetStatusCondition sets the corresponding condition in conditions to newCondition.
func SetStatusCondition(conditions *[]configv1.ClusterOperatorStatusCondition, newCondition configv1.ClusterOperatorStatusCondition) {
if conditions == nil {
conditions = &[]configv1.ClusterOperatorStatusCondition{}
}
existingCondition := FindStatusCondition(*conditions, newCondition.Type)
if existingCondition == nil {
newCondition.LastTransitionTime = metav1.NewTime(time.Now())
*conditions = append(*conditions, newCondition)
return
}

if existingCondition.Status != newCondition.Status {
existingCondition.Status = newCondition.Status
existingCondition.LastTransitionTime = metav1.NewTime(time.Now())
}

existingCondition.Reason = newCondition.Reason
existingCondition.Message = newCondition.Message
}

// RemoveStatusCondition removes the corresponding conditionType from conditions.
func RemoveStatusCondition(conditions *[]configv1.ClusterOperatorStatusCondition, conditionType configv1.ClusterStatusConditionType) {
if conditions == nil {
conditions = &[]configv1.ClusterOperatorStatusCondition{}
}
newConditions := []configv1.ClusterOperatorStatusCondition{}
for _, condition := range *conditions {
if condition.Type != conditionType {
newConditions = append(newConditions, condition)
}
}

*conditions = newConditions
}

// FindStatusCondition finds the conditionType in conditions.
func FindStatusCondition(conditions []configv1.ClusterOperatorStatusCondition, conditionType configv1.ClusterStatusConditionType) *configv1.ClusterOperatorStatusCondition {
func FindStatusCondition(conditions []applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration, conditionType *configv1.ClusterStatusConditionType) *applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration {
for i := range conditions {
if conditions[i].Type == conditionType {
if ptr.Deref(conditions[i].Type, "") == ptr.Deref(conditionType, "") {
return &conditions[i]
}
}
Expand All @@ -62,38 +25,52 @@ func FindStatusCondition(conditions []configv1.ClusterOperatorStatusCondition, c
}

// GetStatusDiff returns a string representing change in condition status in human readable form.
func GetStatusDiff(oldStatus configv1.ClusterOperatorStatus, newStatus configv1.ClusterOperatorStatus) string {
func GetStatusDiff(oldStatus, newStatus *applyconfigv1.ClusterOperatorStatusApplyConfiguration) string {
switch {
case oldStatus == nil && newStatus == nil:
case oldStatus != nil && newStatus == nil:
return "status was removed"
}
messages := []string{}
for _, newCondition := range newStatus.Conditions {
if oldStatus == nil {
messages = append(messages, fmt.Sprintf("%s set to %s (%q)", ptr.Deref(newCondition.Type, ""), ptr.Deref(newCondition.Status, ""), ptr.Deref(newCondition.Message, "")))
continue
}
existingStatusCondition := FindStatusCondition(oldStatus.Conditions, newCondition.Type)
if existingStatusCondition == nil {
messages = append(messages, fmt.Sprintf("%s set to %s (%q)", newCondition.Type, newCondition.Status, newCondition.Message))
messages = append(messages, fmt.Sprintf("%s set to %s (%q)", ptr.Deref(newCondition.Type, ""), ptr.Deref(newCondition.Status, ""), ptr.Deref(newCondition.Message, "")))
continue
}
if existingStatusCondition.Status != newCondition.Status {
messages = append(messages, fmt.Sprintf("%s changed from %s to %s (%q)", existingStatusCondition.Type, existingStatusCondition.Status, newCondition.Status, newCondition.Message))
if ptr.Deref(existingStatusCondition.Status, "") != ptr.Deref(newCondition.Status, "") {
messages = append(messages, fmt.Sprintf("%s changed from %s to %s (%q)", ptr.Deref(existingStatusCondition.Type, ""), ptr.Deref(existingStatusCondition.Status, ""), ptr.Deref(newCondition.Status, ""), ptr.Deref(newCondition.Message, "")))
continue
}
if existingStatusCondition.Message != newCondition.Message {
messages = append(messages, fmt.Sprintf("%s message changed from %q to %q", existingStatusCondition.Type, existingStatusCondition.Message, newCondition.Message))
existingMessage := strings.TrimPrefix(ptr.Deref(existingStatusCondition.Message, ""), "\ufeff")
newMessage := strings.TrimPrefix(ptr.Deref(newCondition.Message, ""), "\ufeff")
if existingMessage != newMessage {
messages = append(messages, fmt.Sprintf("%s message changed from %q to %q", ptr.Deref(existingStatusCondition.Type, ""), existingMessage, newMessage))
}
}
for _, oldCondition := range oldStatus.Conditions {
// This should not happen. It means we removed old condition entirely instead of just changing its status
if c := FindStatusCondition(newStatus.Conditions, oldCondition.Type); c == nil {
messages = append(messages, fmt.Sprintf("%s was removed", oldCondition.Type))
if oldStatus != nil {
for _, oldCondition := range oldStatus.Conditions {
// This should not happen. It means we removed old condition entirely instead of just changing its status
if c := FindStatusCondition(newStatus.Conditions, oldCondition.Type); c == nil {
messages = append(messages, fmt.Sprintf("%s was removed", ptr.Deref(oldCondition.Type, "")))
}
}
}

if !equality.Semantic.DeepEqual(oldStatus.RelatedObjects, newStatus.RelatedObjects) {
messages = append(messages, fmt.Sprintf("status.relatedObjects changed from %q to %q", oldStatus.RelatedObjects, newStatus.RelatedObjects))
}
if !equality.Semantic.DeepEqual(oldStatus.Extension, newStatus.Extension) {
messages = append(messages, fmt.Sprintf("status.extension changed from %q to %q", oldStatus.Extension, newStatus.Extension))
}

if !equality.Semantic.DeepEqual(oldStatus.Versions, newStatus.Versions) {
messages = append(messages, fmt.Sprintf("status.versions changed from %q to %q", oldStatus.Versions, newStatus.Versions))
if oldStatus != nil {
if !equality.Semantic.DeepEqual(oldStatus.RelatedObjects, newStatus.RelatedObjects) {
messages = append(messages, fmt.Sprintf("status.relatedObjects changed from %#v to %#v", oldStatus.RelatedObjects, newStatus.RelatedObjects))
}
if !equality.Semantic.DeepEqual(oldStatus.Extension, newStatus.Extension) {
messages = append(messages, fmt.Sprintf("status.extension changed from %#v to %#v", oldStatus.Extension, newStatus.Extension))
}
if !equality.Semantic.DeepEqual(oldStatus.Versions, newStatus.Versions) {
messages = append(messages, fmt.Sprintf("status.versions changed from %#v to %#v", oldStatus.Versions, newStatus.Versions))
}
}

if len(messages) == 0 {
Expand Down
60 changes: 32 additions & 28 deletions pkg/config/clusteroperator/v1helpers/status_test.go
Original file line number Diff line number Diff line change
@@ -1,82 +1,86 @@
package v1helpers

import (
configv1 "github.com/openshift/api/config/v1"
"k8s.io/utils/ptr"
"reflect"
"strings"
"testing"

configv1 "github.com/openshift/api/config/v1"
applyconfigv1 "github.com/openshift/client-go/config/applyconfigurations/config/v1"
)

func TestGetStatusConditionDiff(t *testing.T) {
tests := []struct {
name string
newConditions []configv1.ClusterOperatorStatusCondition
oldConditions []configv1.ClusterOperatorStatusCondition
newConditions []applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration
oldConditions []applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration
expectedMessages []string
}{
{
name: "new condition",
newConditions: []configv1.ClusterOperatorStatusCondition{
newConditions: []applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionTrue,
Message: "test",
Type: ptr.To(configv1.RetrievedUpdates),
Status: ptr.To(configv1.ConditionTrue),
Message: ptr.To("test"),
},
},
expectedMessages: []string{`RetrievedUpdates set to True ("test")`},
},
{
name: "condition status change",
newConditions: []configv1.ClusterOperatorStatusCondition{
newConditions: []applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionFalse,
Message: "test",
Type: ptr.To(configv1.RetrievedUpdates),
Status: ptr.To(configv1.ConditionFalse),
Message: ptr.To("test"),
},
},
oldConditions: []configv1.ClusterOperatorStatusCondition{
oldConditions: []applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionTrue,
Message: "test",
Type: ptr.To(configv1.RetrievedUpdates),
Status: ptr.To(configv1.ConditionTrue),
Message: ptr.To("test"),
},
},
expectedMessages: []string{`RetrievedUpdates changed from True to False ("test")`},
},
{
name: "condition message change",
newConditions: []configv1.ClusterOperatorStatusCondition{
newConditions: []applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionTrue,
Message: "foo",
Type: ptr.To(configv1.RetrievedUpdates),
Status: ptr.To(configv1.ConditionTrue),
Message: ptr.To("foo"),
},
},
oldConditions: []configv1.ClusterOperatorStatusCondition{
oldConditions: []applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionTrue,
Message: "bar",
Type: ptr.To(configv1.RetrievedUpdates),
Status: ptr.To(configv1.ConditionTrue),
Message: ptr.To("bar"),
},
},
expectedMessages: []string{`RetrievedUpdates message changed from "bar" to "foo"`},
},
{
name: "condition message deleted",
oldConditions: []configv1.ClusterOperatorStatusCondition{
oldConditions: []applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration{
{
Type: configv1.RetrievedUpdates,
Status: configv1.ConditionTrue,
Message: "test",
Type: ptr.To(configv1.RetrievedUpdates),
Status: ptr.To(configv1.ConditionTrue),
Message: ptr.To("test"),
},
},
expectedMessages: []string{"RetrievedUpdates was removed"},
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
result := GetStatusDiff(configv1.ClusterOperatorStatus{Conditions: test.oldConditions}, configv1.ClusterOperatorStatus{Conditions: test.newConditions})
result := GetStatusDiff(
&applyconfigv1.ClusterOperatorStatusApplyConfiguration{Conditions: test.oldConditions},
&applyconfigv1.ClusterOperatorStatusApplyConfiguration{Conditions: test.newConditions})
if !reflect.DeepEqual(test.expectedMessages, strings.Split(result, ",")) {
t.Errorf("expected %#v, got %#v", test.expectedMessages, result)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package apiservercontrollerset
import (
"context"
"fmt"
"k8s.io/utils/clock"
"regexp"
"time"

Expand Down Expand Up @@ -74,6 +75,7 @@ type APIServerControllerSet struct {
name string
operatorClient v1helpers.OperatorClient
eventRecorder events.Recorder
clock clock.PassiveClock

apiServiceController controllerWrapper
auditPolicyController controllerWrapper
Expand All @@ -92,11 +94,13 @@ func NewAPIServerControllerSet(
name string,
operatorClient v1helpers.OperatorClient,
eventRecorder events.Recorder,
clock clock.PassiveClock,
) *APIServerControllerSet {
apiServerControllerSet := &APIServerControllerSet{
name: name,
operatorClient: operatorClient,
eventRecorder: eventRecorder,
clock: clock,
}

return apiServerControllerSet
Expand Down Expand Up @@ -143,6 +147,7 @@ func (cs *APIServerControllerSet) WithClusterOperatorStatusController(
cs.operatorClient,
versionRecorder,
cs.eventRecorder,
cs.clock,
)
for _, opt := range options {
s = opt(s)
Expand Down
18 changes: 10 additions & 8 deletions pkg/operator/status/condition.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package status

import (
"fmt"
applyconfigv1 "github.com/openshift/client-go/config/applyconfigurations/config/v1"
"k8s.io/utils/ptr"
"sort"
"strings"
"time"
Expand Down Expand Up @@ -89,18 +91,18 @@ func UnionCondition(conditionType string, defaultConditionStatus operatorv1.Cond
// on true, but Available merges on false. Thing of it like an anti-default.
//
// If inertia is non-nil, then resist returning a condition with a status opposite the defaultConditionStatus.
func UnionClusterCondition(conditionType configv1.ClusterStatusConditionType, defaultConditionStatus operatorv1.ConditionStatus, inertia Inertia, allConditions ...operatorv1.OperatorCondition) configv1.ClusterOperatorStatusCondition {
func UnionClusterCondition(conditionType configv1.ClusterStatusConditionType, defaultConditionStatus operatorv1.ConditionStatus, inertia Inertia, allConditions ...operatorv1.OperatorCondition) *applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration {
cnd := UnionCondition(string(conditionType), defaultConditionStatus, inertia, allConditions...)
return OperatorConditionToClusterOperatorCondition(cnd)
}

func OperatorConditionToClusterOperatorCondition(condition operatorv1.OperatorCondition) configv1.ClusterOperatorStatusCondition {
return configv1.ClusterOperatorStatusCondition{
Type: configv1.ClusterStatusConditionType(condition.Type),
Status: configv1.ConditionStatus(condition.Status),
LastTransitionTime: condition.LastTransitionTime,
Reason: condition.Reason,
Message: condition.Message,
func OperatorConditionToClusterOperatorCondition(condition operatorv1.OperatorCondition) *applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration {
return &applyconfigv1.ClusterOperatorStatusConditionApplyConfiguration{
Type: ptr.To(configv1.ClusterStatusConditionType(condition.Type)),
Status: ptr.To(configv1.ConditionStatus(condition.Status)),
LastTransitionTime: ptr.To(condition.LastTransitionTime),
Reason: ptr.To(condition.Reason),
Message: ptr.To(condition.Message),
}
}
func latestTransitionTime(conditions []operatorv1.OperatorCondition) metav1.Time {
Expand Down
Loading

0 comments on commit c774bfa

Please sign in to comment.