Skip to content

Commit

Permalink
Merge pull request openshift#16 from RadekManak/condition_refactor
Browse files Browse the repository at this point in the history
[OCPCLOUD-1371] Refactor provider status to use metav1.Condition
  • Loading branch information
openshift-merge-robot authored Mar 31, 2022
2 parents 639eb32 + 4d1d710 commit 242163c
Show file tree
Hide file tree
Showing 17 changed files with 74 additions and 261 deletions.
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ require (
github.com/mitchellh/mapstructure v1.4.2
github.com/onsi/ginkgo v1.16.5
github.com/onsi/gomega v1.17.0
github.com/openshift/api v0.0.0-20220324135351-3d6a78ae0721
github.com/openshift/api v0.0.0-20220325173635-8107b7a38e53
github.com/openshift/machine-api-operator v0.2.1-0.20220327131531-58ba8507d869
github.com/spf13/cobra v1.2.1
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -570,8 +570,8 @@ github.com/opencontainers/go-digest v1.0.0/go.mod h1:0JzlMkj0TRzQZfJkVvzbP0HBR3I
github.com/opencontainers/image-spec v1.0.1/go.mod h1:BtxoFyWECRxE4U/7sNtV5W15zMzWCbyJoFRP3s7yZA0=
github.com/openshift/api v0.0.0-20211209135129-c58d9f695577/go.mod h1:DoslCwtqUpr3d/gsbq4ZlkaMEdYqKxuypsDjorcHhME=
github.com/openshift/api v0.0.0-20220322000322-9c4998a4d646/go.mod h1:F/eU6jgr6Q2VhMu1mSpMmygxAELd7+BUxs3NHZ25jV4=
github.com/openshift/api v0.0.0-20220324135351-3d6a78ae0721 h1:2kDNqCgKxnv/KuvUoDlc9sC0TODOtSOxPvuxvKKkVaM=
github.com/openshift/api v0.0.0-20220324135351-3d6a78ae0721/go.mod h1:F/eU6jgr6Q2VhMu1mSpMmygxAELd7+BUxs3NHZ25jV4=
github.com/openshift/api v0.0.0-20220325173635-8107b7a38e53 h1:WpOfczPbuY5llRH94DJRsGTsgm7d8iAA2b7m0+YXAqI=
github.com/openshift/api v0.0.0-20220325173635-8107b7a38e53/go.mod h1:F/eU6jgr6Q2VhMu1mSpMmygxAELd7+BUxs3NHZ25jV4=
github.com/openshift/build-machinery-go v0.0.0-20210712174854-1bb7fd1518d3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
github.com/openshift/build-machinery-go v0.0.0-20211213093930-7e33a7eb4ce3/go.mod h1:b1BuldmJlbA/xYtdZvKi+7j5YGB44qJUJDZ9zwiNCfE=
github.com/openshift/client-go v0.0.0-20211209144617-7385dd6338e3 h1:SG1aqwleU6bGD0X4mhkTNupjVnByMYYuW4XbnCPavQU=
Expand Down
15 changes: 6 additions & 9 deletions pkg/cloud/azure/actuators/machine/conditions.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package machine

import (
machinev1 "github.com/openshift/api/machine/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"
)
Expand All @@ -13,8 +12,8 @@ const (
)

func shouldUpdateCondition(
oldCondition machinev1.AzureMachineProviderCondition,
newCondition machinev1.AzureMachineProviderCondition,
oldCondition metav1.Condition,
newCondition metav1.Condition,
) bool {
if oldCondition.Status != newCondition.Status ||
oldCondition.Reason != newCondition.Reason ||
Expand All @@ -24,7 +23,7 @@ func shouldUpdateCondition(
return false
}

// setMachineProviderCondition sets the condition for the machine and
// setCondition sets the condition for the machine and
// returns the new slice of conditions.
// If the machine does not already have a condition with the specified type,
// a condition will be added to the slice.
Expand All @@ -33,20 +32,19 @@ func shouldUpdateCondition(
// 1) Requested Status is different than existing status.
// 2) requested Reason is different that existing one.
// 3) requested Message is different that existing one.
func setMachineProviderCondition(conditions []machinev1.AzureMachineProviderCondition, newCondition machinev1.AzureMachineProviderCondition) []machinev1.AzureMachineProviderCondition {
func setCondition(conditions []metav1.Condition, newCondition metav1.Condition) []metav1.Condition {
now := metav1.Now()
currentCondition := findCondition(conditions, newCondition.Type)
if currentCondition == nil {
klog.V(4).Infof("Adding new provider condition %v", newCondition)
conditions = append(
conditions,
machinev1.AzureMachineProviderCondition{
metav1.Condition{
Type: newCondition.Type,
Status: newCondition.Status,
Reason: newCondition.Reason,
Message: newCondition.Message,
LastTransitionTime: now,
LastProbeTime: now,
},
)
} else {
Expand All @@ -61,15 +59,14 @@ func setMachineProviderCondition(conditions []machinev1.AzureMachineProviderCond
currentCondition.Status = newCondition.Status
currentCondition.Reason = newCondition.Reason
currentCondition.Message = newCondition.Message
currentCondition.LastProbeTime = now
}
}
return conditions
}

// findCondition finds in the machine the condition that has the
// specified condition type. If none exists, then returns nil.
func findCondition(conditions []machinev1.AzureMachineProviderCondition, conditionType machinev1.ConditionType) *machinev1.AzureMachineProviderCondition {
func findCondition(conditions []metav1.Condition, conditionType string) *metav1.Condition {
for i, condition := range conditions {
if condition.Type == conditionType {
return &conditions[i]
Expand Down
39 changes: 19 additions & 20 deletions pkg/cloud/azure/actuators/machine/conditions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,65 +3,64 @@ package machine
import (
"testing"

machinev1 "github.com/openshift/api/machine/v1beta1"
corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

func TestShouldUpdateCondition(t *testing.T) {
testCases := []struct {
oldCondition machinev1.AzureMachineProviderCondition
newCondition machinev1.AzureMachineProviderCondition
oldCondition metav1.Condition
newCondition metav1.Condition
expected bool
}{
{
oldCondition: machinev1.AzureMachineProviderCondition{
oldCondition: metav1.Condition{
Reason: "foo",
Message: "bar",
Status: corev1.ConditionTrue,
Status: metav1.ConditionTrue,
},
newCondition: machinev1.AzureMachineProviderCondition{
newCondition: metav1.Condition{
Reason: "foo",
Message: "bar",
Status: corev1.ConditionTrue,
Status: metav1.ConditionTrue,
},
expected: false,
},
{
oldCondition: machinev1.AzureMachineProviderCondition{
oldCondition: metav1.Condition{
Reason: "foo",
Message: "bar",
Status: corev1.ConditionTrue,
Status: metav1.ConditionTrue,
},
newCondition: machinev1.AzureMachineProviderCondition{
newCondition: metav1.Condition{
Reason: "different reason",
Message: "bar",
Status: corev1.ConditionTrue,
Status: metav1.ConditionTrue,
},
expected: true,
},
{
oldCondition: machinev1.AzureMachineProviderCondition{
oldCondition: metav1.Condition{
Reason: "foo",
Message: "different message",
Status: corev1.ConditionTrue,
Status: metav1.ConditionTrue,
},
newCondition: machinev1.AzureMachineProviderCondition{
newCondition: metav1.Condition{
Reason: "foo",
Message: "bar",
Status: corev1.ConditionTrue,
Status: metav1.ConditionTrue,
},
expected: true,
},
{
oldCondition: machinev1.AzureMachineProviderCondition{
oldCondition: metav1.Condition{
Reason: "foo",
Message: "bar",
Status: corev1.ConditionTrue,
Status: metav1.ConditionTrue,
},
newCondition: machinev1.AzureMachineProviderCondition{
newCondition: metav1.Condition{
Reason: "foo",
Message: "bar",
Status: corev1.ConditionFalse,
Status: metav1.ConditionFalse,
},
expected: true,
},
Expand Down
13 changes: 7 additions & 6 deletions pkg/cloud/azure/actuators/machine/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import (
"github.com/openshift/machine-api-provider-azure/pkg/cloud/azure/services/virtualmachineextensions"
"github.com/openshift/machine-api-provider-azure/pkg/cloud/azure/services/virtualmachines"
apicorev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/klog/v2"

"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -95,9 +96,9 @@ func NewReconciler(scope *actuators.MachineScope) *Reconciler {
// Create creates machine if and only if machine exists, handled by cluster-api
func (s *Reconciler) Create(ctx context.Context) error {
if err := s.CreateMachine(ctx); err != nil {
s.scope.MachineStatus.Conditions = setMachineProviderCondition(s.scope.MachineStatus.Conditions, machinev1.AzureMachineProviderCondition{
Type: machinev1.MachineCreated,
Status: apicorev1.ConditionTrue,
s.scope.MachineStatus.Conditions = setCondition(s.scope.MachineStatus.Conditions, metav1.Condition{
Type: string(machinev1.MachineCreated),
Status: metav1.ConditionTrue,
Reason: machineCreationFailedReason,
Message: err.Error(),
})
Expand Down Expand Up @@ -265,9 +266,9 @@ func (s *Reconciler) Update(ctx context.Context) error {
}

// Set instance conditions
s.scope.MachineStatus.Conditions = setMachineProviderCondition(s.scope.MachineStatus.Conditions, machinev1.AzureMachineProviderCondition{
Type: machinev1.MachineCreated,
Status: apicorev1.ConditionTrue,
s.scope.MachineStatus.Conditions = setCondition(s.scope.MachineStatus.Conditions, metav1.Condition{
Type: string(machinev1.MachineCreated),
Status: metav1.ConditionTrue,
Reason: machineCreationSucceedReason,
Message: machineCreationSucceedMessage,
})
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 242163c

Please sign in to comment.