fluxcd
/
source-controller
mirror of https://github.com/fluxcd/source-controller.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

summarize: consider bipolarity in status condition 

This introduces the consideration of bipolarity conditions in the status
condition summary for Ready condition. The summarize.HelperOptions can
now be configured with a list of bipolarity conditions which are used in
SummarizeAndPatch() to set the Ready condition to failing bipolarity
condition with the highest priority.

Bipolarity condition is not a typical status property. It is a mix of
positive and negative polarities. It's "normal-true" and
"abnormal-false". Failing bipolarity conditions are prioritized over
other conditions to show the actual reason of failure on the Ready
status.

Signed-off-by: Sunny <darkowlzz@protonmail.com>
This commit is contained in:
Sunny 2022-09-22 03:27:53 +05:30
parent c9a5a56cfb
commit e5d3aa3701
2 changed files with 111 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
internal/reconcile/summarize/summary.go
View File

@ -90,6 +90,9 @@ type HelperOptions struct {
// PatchFieldOwner defines the field owner configuration for the Kubernetes
// patch operation.
PatchFieldOwner string
// BiPolarityConditionTypes is a list of bipolar conditions in the order
// of priority.
BiPolarityConditionTypes []string
}
// Option is configuration that modifies SummarizeAndPatch.
@ -149,6 +152,14 @@ func WithPatchFieldOwner(fieldOwner string) Option {
}
}
// WithBiPolarityConditionTypes sets the BiPolarityConditionTypes used to
// calculate the value of Ready condition in SummarizeAndPatch.
func WithBiPolarityConditionTypes(types ...string) Option {
return func(s *HelperOptions) {
s.BiPolarityConditionTypes = types
}
}
// SummarizeAndPatch summarizes and patches the result to the target object.
// When used at the very end of a reconciliation, the result builder must be
// specified using the Option WithResultBuilder(). The returned result and error
@ -206,6 +217,26 @@ func (h *Helper) SummarizeAndPatch(ctx context.Context, obj conditions.Setter, o
)
}
// Check any BiPolarity conditions in the status that are False. Failing
// BiPolarity condition should be set as the Ready condition value to
// reflect the actual cause of the reconciliation failure.
// NOTE: This is applicable to Ready condition only because it is a special
// condition in kstatus that reflects the overall state of an object.
// IMPLEMENTATION NOTE: An implementation of this within the
// conditions.merge() exists in fluxcd/pkg repo branch `bipolarity`
// (https://github.com/fluxcd/pkg/commit/756b9e6d253a4fae93c05419b7019d0169454858).
// If that gets added to conditions.merge, the following can be removed.
var failedBiPolarity []string
for _, c := range opts.BiPolarityConditionTypes {
if conditions.IsFalse(obj, c) {
failedBiPolarity = append(failedBiPolarity, c)
}
}
if len(failedBiPolarity) > 0 {
topFailedBiPolarity := conditions.Get(obj, failedBiPolarity[0])
conditions.MarkFalse(obj, meta.ReadyCondition, topFailedBiPolarity.Reason, topFailedBiPolarity.Message)
}
// If object is not stalled, result is success and runtime error is nil,
// ensure that Ready=True. Else, use the Ready failure message as the
// runtime error message. This ensures that the reconciliation would be

89
internal/reconcile/summarize/summary_test.go
View File

@ -44,11 +44,16 @@ import (
// This tests the scenario where SummarizeAndPatch is used at the very end of a
// reconciliation.
func TestSummarizeAndPatch(t *testing.T) {
testBipolarCondition1 := "FooChecked1"
testBipolarCondition2 := "FooChecked2"
var testReadyConditions = Conditions{
Target: meta.ReadyCondition,
Owned: []string{
sourcev1.FetchFailedCondition,
sourcev1.ArtifactOutdatedCondition,
sourcev1.SourceVerifiedCondition,
testBipolarCondition1,
testBipolarCondition2,
meta.ReadyCondition,
meta.ReconcilingCondition,
meta.StalledCondition,
@ -56,6 +61,9 @@ func TestSummarizeAndPatch(t *testing.T) {
Summarize: []string{
sourcev1.FetchFailedCondition,
sourcev1.ArtifactOutdatedCondition,
sourcev1.SourceVerifiedCondition,
testBipolarCondition1,
testBipolarCondition2,
meta.StalledCondition,
meta.ReconcilingCondition,
},
@ -66,6 +74,7 @@ func TestSummarizeAndPatch(t *testing.T) {
meta.ReconcilingCondition,
},
}
var testBipolarConditions = []string{sourcev1.SourceVerifiedCondition, testBipolarCondition1, testBipolarCondition2}
var testFooConditions = Conditions{
Target: "Foo",
Owned: []string{
@ -83,15 +92,16 @@ func TestSummarizeAndPatch(t *testing.T) {
}
tests := []struct {
name string
generation int64
beforeFunc func(obj conditions.Setter)
result reconcile.Result
reconcileErr error
conditions []Conditions
wantErr bool
afterFunc func(t *WithT, obj client.Object)
assertConditions []metav1.Condition
name string
generation int64
beforeFunc func(obj conditions.Setter)
result reconcile.Result
reconcileErr error
conditions []Conditions
bipolarConditions []string
wantErr bool
afterFunc func(t *WithT, obj client.Object)
assertConditions []metav1.Condition
}{
// Success/Fail indicates if a reconciliation succeeded or failed.
// The object generation is expected to match the observed generation in
@ -250,6 +260,64 @@ func TestSummarizeAndPatch(t *testing.T) {
},
wantErr: true,
},
{
name: "Fail, reconciling with bipolar condition False, Ready gets bipolar failure value",
generation: 2,
beforeFunc: func(obj conditions.Setter) {
conditions.MarkReconciling(obj, "NewRevision", "new index revision")
conditions.MarkFalse(obj, sourcev1.SourceVerifiedCondition, "VerifyFailed", "verify failed")
},
result: reconcile.ResultEmpty,
reconcileErr: errors.New("failed to verify source"),
conditions: []Conditions{testReadyConditions},
bipolarConditions: testBipolarConditions,
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.FalseCondition(meta.ReadyCondition, "VerifyFailed", "verify failed"),
*conditions.FalseCondition(sourcev1.SourceVerifiedCondition, "VerifyFailed", "verify failed"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewRevision", "new index revision"),
},
},
{
name: "Fail, bipolar condition True, negative polarity True, Ready gets negative polarity value",
generation: 2,
beforeFunc: func(obj conditions.Setter) {
conditions.MarkReconciling(obj, "NewGeneration", "new obj gen")
conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "new digest")
conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, "Success", "verified")
},
result: reconcile.ResultEmpty,
reconcileErr: errors.New("failed to create dir"),
conditions: []Conditions{testReadyConditions},
bipolarConditions: testBipolarConditions,
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.FalseCondition(meta.ReadyCondition, "NewRevision", "new digest"),
*conditions.TrueCondition(sourcev1.ArtifactOutdatedCondition, "NewRevision", "new digest"),
*conditions.TrueCondition(meta.ReconcilingCondition, "NewGeneration", "new obj gen"),
*conditions.TrueCondition(sourcev1.SourceVerifiedCondition, "Success", "verified"),
},
},
{
name: "Fail, multiple bipolar conditions False, Ready gets the bipolar with high priority",
generation: 2,
beforeFunc: func(obj conditions.Setter) {
conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, "Success", "verified")
conditions.MarkFalse(obj, testBipolarCondition1, "AAA", "aaa")
conditions.MarkFalse(obj, testBipolarCondition2, "BBB", "bbb")
},
result: reconcile.ResultEmpty,
reconcileErr: errors.New("some failure"),
conditions: []Conditions{testReadyConditions},
bipolarConditions: testBipolarConditions,
wantErr: true,
assertConditions: []metav1.Condition{
*conditions.FalseCondition(meta.ReadyCondition, "AAA", "aaa"),
*conditions.FalseCondition(testBipolarCondition1, "AAA", "aaa"),
*conditions.FalseCondition(testBipolarCondition2, "BBB", "bbb"),
*conditions.TrueCondition(sourcev1.SourceVerifiedCondition, "Success", "verified"),
},
},
}
for _, tt := range tests {
@ -289,6 +357,9 @@ func TestSummarizeAndPatch(t *testing.T) {
WithProcessors(RecordContextualError, RecordReconcileReq),
WithResultBuilder(reconcile.AlwaysRequeueResultBuilder{RequeueAfter: obj.Spec.Interval.Duration}),
}
if tt.bipolarConditions != nil {
summaryOpts = append(summaryOpts, WithBiPolarityConditionTypes(tt.bipolarConditions...))
}
_, gotErr := summaryHelper.SummarizeAndPatch(ctx, obj, summaryOpts...)
g.Expect(gotErr != nil).To(Equal(tt.wantErr))