Skip to content

Commit 4f1637e

Browse files
fabriziopandinifabriziopandini
committed
Rename helpers
1 parent bb4f597 commit 4f1637e

File tree

6 files changed

+61
-57
lines changed

6 files changed

+61
-57
lines changed

‎docs/proposals/20200506-conditions.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,8 +243,9 @@ When the above suffix are not adequate for a specific condition type, other suff
243243
(e.g. `Completed`, `Healthy`); however, it is recommended to balance this flexibility with the objective to provide
244244
a consistent condition naming across all the Cluster API objects.
245245

246-
The `Severity` field MUST be set only when `Status=False` + positive polarity or when `Status=True` + negative polarity;
247-
severity it is designed to provide a standard classification of possible conditions failure `Reason`.
246+
The `Severity` field MUST be set only when `Status=False` for conditions with positive polarity, or when `Status=True`
247+
for conditions with negative polarity; severity is designed to provide a standard classification of possible
248+
conditions failure `Reason`.
248249

249250
Please note that the combination of `Reason` and `Severity` gives different meaning to a condition failure
250251
allowing to detect when a long-running task is still ongoing:
@@ -473,7 +474,7 @@ enhance the condition utilities to handle those situations in a generalized way.
473474
- Risk: This proposal aims to be consistent with the target state of conditions in Kubernetes, but this
474475
is still under definition (see [KEP](https://github.com/kubernetes/enhancements/pull/1624)).
475476
- Mitigation: Periodically re-evaluate this proposal vs the Kubernetes KEP.
476-
- 2024-05-03: Change allowing conditions with negative polarity goes into this direction
477+
- 2024-05-03: Change to allow conditions without positive polarity goes into this direction
477478

478479
- Risk: Cluster API presents some specific challenges that are not common to the core Kubernetes objects.
479480
- Mitigation: To allow a minimal set of carefully evaluated differences between Cluster API and Kubernetes

‎util/conditions/getter_test.go

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ var (
3333
falseWarning1 = FalseCondition("falseWarning1", "reason falseWarning1", clusterv1.ConditionSeverityWarning, "message falseWarning1")
3434
falseError1 = FalseCondition("falseError1", "reason falseError1", clusterv1.ConditionSeverityError, "message falseError1")
3535

36-
negativePolarityConditions = sets.New("positive-false1", "negative-unknown1", "negative-trueInfo1", "negative-trueWarning1", "negative-trueError1")
37-
positiveFalse1 = PositiveFalseCondition("positive-false1")
38-
negativeUnknown1 = UnknownCondition("negative-unknown1", "reason negative-unknown1", "message negative-unknown1")
39-
negativeTrueInfo1 = NegativeTrueCondition("negative-trueInfo1", "reason negative-trueInfo1", clusterv1.ConditionSeverityInfo, "message negative-trueInfo1")
40-
negativeTrueWarning1 = NegativeTrueCondition("negative-trueWarning1", "reason negative-trueWarning1", clusterv1.ConditionSeverityWarning, "message negative-trueWarning1")
41-
negativeTrueError1 = NegativeTrueCondition("negative-trueError1", "reason negative-trueError1", clusterv1.ConditionSeverityError, "message negative-trueError1")
36+
negativePolarityConditions = sets.New("false1-negative-polarity", "unknown1-negative-polarity", "trueInfo1-negative-polarity", "trueWarning1-negative-polarity", "trueError1-negative-polarity")
37+
false1WithNegativePolarity = FalseConditionWithNegativePolarity("false1-negative-polarity")
38+
unknown1WithNegativePolarity = UnknownCondition("unknown1-negative-polarity", "reason unknown1-negative-polarity", "message unknown1-negative-polarity")
39+
trueInfo1WithNegativePolarity = TrueConditionWithNegativePolarity("trueInfo1-negative-polarity", "reason trueInfo1-negative-polarity", clusterv1.ConditionSeverityInfo, "message trueInfo1-negative-polarity")
40+
trueWarning1WithNegativePolarity = TrueConditionWithNegativePolarity("trueWarning1-negative-polarity", "reason trueWarning1-negative-polarity", clusterv1.ConditionSeverityWarning, "message trueWarning1-negative-polarity")
41+
trueError1WithNegativePolarity = TrueConditionWithNegativePolarity("trueError1-negative-polarity", "reason trueError1-negative-polarity", clusterv1.ConditionSeverityError, "message trueError1-negative-polarity")
4242
)
4343

4444
func TestGetAndHas(t *testing.T) {
@@ -58,51 +58,51 @@ func TestGetAndHas(t *testing.T) {
5858
func TestIsMethods(t *testing.T) {
5959
g := NewWithT(t)
6060

61-
obj := getterWithConditions(nil1, true1, unknown1, falseInfo1, falseWarning1, falseError1, positiveFalse1, negativeUnknown1, negativeTrueInfo1, negativeTrueWarning1, negativeTrueError1)
61+
obj := getterWithConditions(nil1, true1, unknown1, falseInfo1, falseWarning1, falseError1, false1WithNegativePolarity, unknown1WithNegativePolarity, trueInfo1WithNegativePolarity, trueWarning1WithNegativePolarity, trueError1WithNegativePolarity)
6262

6363
// test isTrue
6464
g.Expect(IsTrue(obj, "nil1")).To(BeFalse())
6565
g.Expect(IsTrue(obj, "true1")).To(BeTrue())
6666
g.Expect(IsTrue(obj, "falseInfo1")).To(BeFalse())
6767
g.Expect(IsTrue(obj, "unknown1")).To(BeFalse())
68-
g.Expect(IsTrue(obj, "positive-false1")).To(BeFalse())
69-
g.Expect(IsTrue(obj, "negative-trueInfo1")).To(BeTrue())
70-
g.Expect(IsTrue(obj, "negative-unknown1")).To(BeFalse())
68+
g.Expect(IsTrue(obj, "false1-negative-polarity")).To(BeFalse())
69+
g.Expect(IsTrue(obj, "trueInfo1-negative-polarity")).To(BeTrue())
70+
g.Expect(IsTrue(obj, "unknown1-negative-polarity")).To(BeFalse())
7171

7272
// test isFalse
7373
g.Expect(IsFalse(obj, "nil1")).To(BeFalse())
7474
g.Expect(IsFalse(obj, "true1")).To(BeFalse())
7575
g.Expect(IsFalse(obj, "falseInfo1")).To(BeTrue())
7676
g.Expect(IsFalse(obj, "unknown1")).To(BeFalse())
77-
g.Expect(IsFalse(obj, "positive-false1")).To(BeTrue())
78-
g.Expect(IsFalse(obj, "negative-trueInfo1")).To(BeFalse())
79-
g.Expect(IsFalse(obj, "negative-unknown1")).To(BeFalse())
77+
g.Expect(IsFalse(obj, "false1-negative-polarity")).To(BeTrue())
78+
g.Expect(IsFalse(obj, "trueInfo1-negative-polarity")).To(BeFalse())
79+
g.Expect(IsFalse(obj, "unknown1-negative-polarity")).To(BeFalse())
8080

8181
// test isUnknown
8282
g.Expect(IsUnknown(obj, "nil1")).To(BeTrue())
8383
g.Expect(IsUnknown(obj, "true1")).To(BeFalse())
8484
g.Expect(IsUnknown(obj, "falseInfo1")).To(BeFalse())
8585
g.Expect(IsUnknown(obj, "unknown1")).To(BeTrue())
86-
g.Expect(IsUnknown(obj, "positive-false1")).To(BeFalse())
87-
g.Expect(IsUnknown(obj, "negative-trueInfo1")).To(BeFalse())
88-
g.Expect(IsUnknown(obj, "negative-unknown1")).To(BeTrue())
86+
g.Expect(IsUnknown(obj, "false1-negative-polarity")).To(BeFalse())
87+
g.Expect(IsUnknown(obj, "trueInfo1-negative-polarity")).To(BeFalse())
88+
g.Expect(IsUnknown(obj, "unknown1-negative-polarity")).To(BeTrue())
8989

9090
// test GetReason
9191
g.Expect(GetReason(obj, "nil1")).To(Equal(""))
9292
g.Expect(GetReason(obj, "falseInfo1")).To(Equal("reason falseInfo1"))
93-
g.Expect(GetReason(obj, "negative-trueInfo1")).To(Equal("reason negative-trueInfo1"))
93+
g.Expect(GetReason(obj, "trueInfo1-negative-polarity")).To(Equal("reason trueInfo1-negative-polarity"))
9494

9595
// test GetMessage
9696
g.Expect(GetMessage(obj, "nil1")).To(Equal(""))
9797
g.Expect(GetMessage(obj, "falseInfo1")).To(Equal("message falseInfo1"))
98-
g.Expect(GetMessage(obj, "negative-trueInfo1")).To(Equal("message negative-trueInfo1"))
98+
g.Expect(GetMessage(obj, "trueInfo1-negative-polarity")).To(Equal("message trueInfo1-negative-polarity"))
9999

100100
// test GetSeverity
101101
expectedSeverity := clusterv1.ConditionSeverityInfo
102102
g.Expect(GetSeverity(obj, "nil1")).To(BeNil())
103103
severity := GetSeverity(obj, "falseInfo1")
104104
g.Expect(severity).To(Equal(&expectedSeverity))
105-
severity = GetSeverity(obj, "negative-trueInfo1")
105+
severity = GetSeverity(obj, "trueInfo1-negative-polarity")
106106
g.Expect(severity).To(Equal(&expectedSeverity))
107107

108108
// test GetLastTransitionTime
@@ -153,8 +153,8 @@ func TestSummary(t *testing.T) {
153153
foo := TrueCondition("foo")
154154
bar := FalseCondition("bar", "reason falseInfo1", clusterv1.ConditionSeverityInfo, "message falseInfo1")
155155
baz := FalseCondition("baz", "reason falseInfo2", clusterv1.ConditionSeverityInfo, "message falseInfo2")
156-
negativeFoo := PositiveFalseCondition("negative-foo")
157-
negativeBar := NegativeTrueCondition("negative-bar", "reason negative-falseInfo1", clusterv1.ConditionSeverityInfo, "message negative-falseInfo1")
156+
fooWithNegativePolarity := FalseConditionWithNegativePolarity("foo-negative-polarity")
157+
barWithNegativePolarity := TrueConditionWithNegativePolarity("bar-negative-polarity", "reason trueInfo1-negative-polarity", clusterv1.ConditionSeverityInfo, "message trueInfo1-negative-polarity")
158158
existingReady := FalseCondition(clusterv1.ReadyCondition, "reason falseError1", clusterv1.ConditionSeverityError, "message falseError1") // NB. existing ready has higher priority than other conditions
159159

160160
tests := []struct {
@@ -175,15 +175,15 @@ func TestSummary(t *testing.T) {
175175
},
176176
{
177177
name: "Returns ready condition with the summary of existing conditions with negative polarity (with default options)",
178-
from: getterWithConditions(negativeFoo, negativeBar),
179-
options: []MergeOption{WithNegativePolarityConditions("negative-foo", "negative-bar")},
180-
want: FalseCondition(clusterv1.ReadyCondition, "reason negative-falseInfo1", clusterv1.ConditionSeverityInfo, "message negative-falseInfo1"),
178+
from: getterWithConditions(fooWithNegativePolarity, barWithNegativePolarity),
179+
options: []MergeOption{WithNegativePolarityConditions("foo-negative-polarity", "bar-negative-polarity")},
180+
want: FalseCondition(clusterv1.ReadyCondition, "reason trueInfo1-negative-polarity", clusterv1.ConditionSeverityInfo, "message trueInfo1-negative-polarity"),
181181
},
182182
{
183183
name: "Returns ready condition with the summary of existing conditions with mixed polarity (with default options)",
184-
from: getterWithConditions(foo, bar, negativeFoo, negativeBar),
185-
options: []MergeOption{WithNegativePolarityConditions("negative-foo", "negative-bar")},
186-
want: FalseCondition(clusterv1.ReadyCondition, "reason falseInfo1", clusterv1.ConditionSeverityInfo, "message falseInfo1"), // bar take precedence on negativeBar because it is listed first
184+
from: getterWithConditions(foo, bar, fooWithNegativePolarity, barWithNegativePolarity),
185+
options: []MergeOption{WithNegativePolarityConditions("foo-negative-polarity", "bar-negative-polarity")},
186+
want: FalseCondition(clusterv1.ReadyCondition, "reason falseInfo1", clusterv1.ConditionSeverityInfo, "message falseInfo1"), // bar take precedence on barWithNegativePolarity because it is listed first
187187
},
188188
{
189189
name: "Returns ready condition with the summary of existing conditions (using WithStepCounter options)",
@@ -223,15 +223,15 @@ func TestSummary(t *testing.T) {
223223
},
224224
{
225225
name: "Returns ready condition with the summary of selected conditions with negative polarity (using WithConditions options)",
226-
from: getterWithConditions(negativeFoo, negativeBar),
227-
options: []MergeOption{WithConditions("negative-foo"), WithNegativePolarityConditions("negative-foo", "negative-bar")}, // negative-bar should be ignored
226+
from: getterWithConditions(fooWithNegativePolarity, barWithNegativePolarity),
227+
options: []MergeOption{WithConditions("foo-negative-polarity"), WithNegativePolarityConditions("foo-negative-polarity", "bar-negative-polarity")}, // bar-negative-polarity should be ignored because it is not listed in WithConditions
228228
want: TrueCondition(clusterv1.ReadyCondition),
229229
},
230230
{
231231
name: "Returns ready condition with the summary of selected conditions with mixed polarity (using WithConditions options)",
232-
from: getterWithConditions(foo, bar, negativeFoo, negativeBar),
233-
options: []MergeOption{WithConditions("foo", "negative-foo", "negative-bar"), WithNegativePolarityConditions("negative-foo", "negative-bar")}, // bar should be ignored
234-
want: FalseCondition(clusterv1.ReadyCondition, "reason negative-falseInfo1", clusterv1.ConditionSeverityInfo, "message negative-falseInfo1"),
232+
from: getterWithConditions(foo, bar, fooWithNegativePolarity, barWithNegativePolarity),
233+
options: []MergeOption{WithConditions("foo", "foo-negative-polarity", "bar-negative-polarity"), WithNegativePolarityConditions("foo-negative-polarity", "bar-negative-polarity")},
234+
want: FalseCondition(clusterv1.ReadyCondition, "reason trueInfo1-negative-polarity", clusterv1.ConditionSeverityInfo, "message trueInfo1-negative-polarity"),
235235
},
236236
{
237237
name: "Returns ready condition with the summary of selected conditions (using WithConditions and WithStepCounter options)",

‎util/conditions/merge_strategies.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ func WithConditions(t ...clusterv1.ConditionType) MergeOption {
5252
}
5353

5454
// WithNegativePolarityConditions instruct merge about which conditions should be considered having negative polarity.
55+
// IMPORTANT: this must be a subset of WithConditions.
5556
func WithNegativePolarityConditions(t ...clusterv1.ConditionType) MergeOption {
5657
return func(c *mergeOptions) {
5758
c.negativeConditionTypes = t

‎util/conditions/merge_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,12 @@ func TestNewConditionsGroup(t *testing.T) {
8686
t.Run("Negative polarity", func(t *testing.T) {
8787
g := NewWithT(t)
8888

89-
conditions := []*clusterv1.Condition{nil1, positiveFalse1, positiveFalse1, negativeTrueInfo1, negativeTrueWarning1, negativeTrueWarning1, negativeTrueError1, negativeUnknown1}
89+
conditions := []*clusterv1.Condition{nil1, false1WithNegativePolarity, false1WithNegativePolarity, trueInfo1WithNegativePolarity, trueWarning1WithNegativePolarity, trueWarning1WithNegativePolarity, trueError1WithNegativePolarity, unknown1WithNegativePolarity}
9090

9191
got := getConditionGroups(conditionsWithSource(&clusterv1.Cluster{}, conditions...))
9292

93+
// NOTE: groups always have a positive polarity
94+
9395
g.Expect(got).ToNot(BeNil())
9496
g.Expect(got).To(HaveLen(5))
9597

@@ -143,7 +145,7 @@ func TestNewConditionsGroup(t *testing.T) {
143145
t.Run("Mixed polarity", func(t *testing.T) {
144146
g := NewWithT(t)
145147

146-
conditions := []*clusterv1.Condition{nil1, true1, true1, falseInfo1, falseWarning1, falseWarning1, falseError1, unknown1, positiveFalse1, positiveFalse1, negativeTrueInfo1, negativeTrueWarning1, negativeTrueWarning1, negativeTrueError1, negativeUnknown1}
148+
conditions := []*clusterv1.Condition{nil1, true1, true1, falseInfo1, falseWarning1, falseWarning1, falseError1, unknown1, false1WithNegativePolarity, false1WithNegativePolarity, trueInfo1WithNegativePolarity, trueWarning1WithNegativePolarity, trueWarning1WithNegativePolarity, trueError1WithNegativePolarity, unknown1WithNegativePolarity}
147149

148150
got := getConditionGroups(conditionsWithSource(&clusterv1.Cluster{}, conditions...))
149151

‎util/conditions/setter.go

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -85,30 +85,30 @@ func TrueCondition(t clusterv1.ConditionType) *clusterv1.Condition {
8585
}
8686
}
8787

88-
// FalseCondition returns a condition with Status=False and the given type.
89-
func FalseCondition(t clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity, messageFormat string, messageArgs ...interface{}) *clusterv1.Condition {
88+
// TrueConditionWithNegativePolarity returns a condition with negative polarity, Status=True and the given type (Status=True has a negative meaning).
89+
func TrueConditionWithNegativePolarity(t clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity, messageFormat string, messageArgs ...interface{}) *clusterv1.Condition {
9090
return &clusterv1.Condition{
9191
Type: t,
92-
Status: corev1.ConditionFalse,
92+
Status: corev1.ConditionTrue,
9393
Reason: reason,
9494
Severity: severity,
9595
Message: fmt.Sprintf(messageFormat, messageArgs...),
9696
}
9797
}
9898

99-
// NegativeTrueCondition returns a negative polarity condition with Status=True and the given type (Status=True has a negative meaning).
100-
func NegativeTrueCondition(t clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity, messageFormat string, messageArgs ...interface{}) *clusterv1.Condition {
99+
// FalseCondition returns a condition with Status=False and the given type.
100+
func FalseCondition(t clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity, messageFormat string, messageArgs ...interface{}) *clusterv1.Condition {
101101
return &clusterv1.Condition{
102102
Type: t,
103-
Status: corev1.ConditionTrue,
103+
Status: corev1.ConditionFalse,
104104
Reason: reason,
105105
Severity: severity,
106106
Message: fmt.Sprintf(messageFormat, messageArgs...),
107107
}
108108
}
109109

110-
// PositiveFalseCondition returns a negative polarity condition with Status=false and the given type (Status=False has a positive meaning).
111-
func PositiveFalseCondition(t clusterv1.ConditionType) *clusterv1.Condition {
110+
// FalseConditionWithNegativePolarity returns a condition with negative polarity, Status=false and the given type (Status=False has a positive meaning).
111+
func FalseConditionWithNegativePolarity(t clusterv1.ConditionType) *clusterv1.Condition {
112112
return &clusterv1.Condition{
113113
Type: t,
114114
Status: corev1.ConditionFalse,
@@ -130,6 +130,11 @@ func MarkTrue(to Setter, t clusterv1.ConditionType) {
130130
Set(to, TrueCondition(t))
131131
}
132132

133+
// MarkTrueWithNegativePolarity sets Status=True for a condition with negative polarity and the given type (Status=True has a negative meaning).
134+
func MarkTrueWithNegativePolarity(to Setter, t clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity, messageFormat string, messageArgs ...interface{}) {
135+
Set(to, TrueConditionWithNegativePolarity(t, reason, severity, messageFormat, messageArgs...))
136+
}
137+
133138
// MarkUnknown sets Status=Unknown for the condition with the given type.
134139
func MarkUnknown(to Setter, t clusterv1.ConditionType, reason, messageFormat string, messageArgs ...interface{}) {
135140
Set(to, UnknownCondition(t, reason, messageFormat, messageArgs...))
@@ -140,14 +145,9 @@ func MarkFalse(to Setter, t clusterv1.ConditionType, reason string, severity clu
140145
Set(to, FalseCondition(t, reason, severity, messageFormat, messageArgs...))
141146
}
142147

143-
// MarkNegativeTrue sets Status=True for the negative polarity condition with the given type (Status=True has a negative meaning).
144-
func MarkNegativeTrue(to Setter, t clusterv1.ConditionType, reason string, severity clusterv1.ConditionSeverity, messageFormat string, messageArgs ...interface{}) {
145-
Set(to, NegativeTrueCondition(t, reason, severity, messageFormat, messageArgs...))
146-
}
147-
148-
// MarkPositiveFalse sets Status=False for the negative polarity condition with the given type (Status=False has a positive meaning).
149-
func MarkPositiveFalse(to Setter, t clusterv1.ConditionType) {
150-
Set(to, PositiveFalseCondition(t))
148+
// MarkFalseWithNegativePolarity sets Status=False for a condition with negative polarity and the given type (Status=False has a positive meaning).
149+
func MarkFalseWithNegativePolarity(to Setter, t clusterv1.ConditionType) {
150+
Set(to, FalseConditionWithNegativePolarity(t))
151151
}
152152

153153
// SetSummary sets a Ready condition with the summary of all the conditions existing

‎util/conditions/setter_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,15 +224,15 @@ func TestMarkMethods(t *testing.T) {
224224
Message: "messageBaz",
225225
}))
226226

227-
// test MarkPositiveFalse
228-
MarkPositiveFalse(cluster, "conditionFoo")
227+
// test MarkFalseWithNegativePolarity
228+
MarkFalseWithNegativePolarity(cluster, "conditionFoo")
229229
g.Expect(Get(cluster, "conditionFoo")).To(HaveSameStateOf(&clusterv1.Condition{
230230
Type: "conditionFoo",
231231
Status: corev1.ConditionFalse,
232232
}))
233233

234-
// test MarkNegativeTrue
235-
MarkNegativeTrue(cluster, "conditionBar", "reasonBar", clusterv1.ConditionSeverityError, "messageBar")
234+
// test MarkTrueWithNegativePolarity
235+
MarkTrueWithNegativePolarity(cluster, "conditionBar", "reasonBar", clusterv1.ConditionSeverityError, "messageBar")
236236
g.Expect(Get(cluster, "conditionBar")).To(HaveSameStateOf(&clusterv1.Condition{
237237
Type: "conditionBar",
238238
Status: corev1.ConditionTrue,

0 commit comments

Comments
 (0)