Skip to content

Commit 029d19f

Browse files
authored
(fix) Remove "Serving" condition type from ConditionSets (#1859)
conditionset.ConditionTypes was being used by the CluterExtension controller to `ensureAllConditionsWithReason` whenever a particular reason needed to be set for all of the ClusterExention's conditions. However, the `ConditionTypes` included a Condition Type `Serving`, which is not a ClusterExtension Condition(it is a ClusterCatalog condition). This is causing the `Serving` condition to show up when a resolution fails, which is incorrect to begin with, and is then never cleared when the resolution succeeds at a later stage. ``` try to upgrade ClusterExtension to a non-exist version $ kubectl patch ClusterExtension extension-77972 -p '{"spec":{"source":{"catalog":{"version":"0.2.0"}}}}' --type=merge clusterextension.olm.operatorframework.io/extension-77972 patched - lastTransitionTime: "2025-03-04T07:16:27Z" message: 'error upgrading from currently installed version "0.1.0": no bundles found for package "nginx77972" matching version "0.2.0"' observedGeneration: 2 reason: Retrying status: "True" type: Progressing - lastTransitionTime: "2025-03-04T07:16:35Z" message: 'error upgrading from currently installed version "0.1.0": no bundles found for package "nginx77972" matching version "0.2.0"' observedGeneration: 2 reason: Failed status: "False" type: Serving install: bundle: name: nginx77972.v0.1.0 version: 0.1.0 ``` This PR removes the `Serving` condition type from `conditonSets.ConditionType`
1 parent 995dc2b commit 029d19f

File tree

4 files changed

+38
-43
lines changed

4 files changed

+38
-43
lines changed

api/v1/clustercatalog_types.go

+8
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,14 @@ const (
3434

3535
AvailabilityModeAvailable AvailabilityMode = "Available"
3636
AvailabilityModeUnavailable AvailabilityMode = "Unavailable"
37+
38+
// Condition types
39+
TypeServing = "Serving"
40+
41+
// Serving Reasons
42+
ReasonAvailable = "Available"
43+
ReasonUnavailable = "Unavailable"
44+
ReasonUserSpecifiedUnavailable = "UserSpecifiedUnavailable"
3745
)
3846

3947
//+kubebuilder:object:root=true

api/v1/clusterextension_types_test.go

+30-33
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"go/ast"
66
"go/parser"
77
"go/token"
8-
"io/fs"
98
"strconv"
109
"strings"
1110
"testing"
@@ -52,49 +51,47 @@ func TestClusterExtensionReasonRegistration(t *testing.T) {
5251
}
5352
}
5453

55-
// parseConstants parses the values of the top-level constants in the current
56-
// directory whose names start with the given prefix. When running as part of a
57-
// test, the current directory is the directory of the file that contains the
58-
// test in which this function is called.
54+
// parseConstants parses the values of the top-level constants that start with the given prefix,
55+
// in the files clusterextension_types.go and common_types.go.
5956
func parseConstants(prefix string) ([]string, error) {
6057
fset := token.NewFileSet()
61-
// ParseDir returns a map of package name to package ASTs. An AST is a representation of the source code
62-
// that can be traversed to extract information. The map is keyed by the package name.
63-
pkgs, err := parser.ParseDir(fset, ".", func(info fs.FileInfo) bool {
64-
return !strings.HasSuffix(info.Name(), "_test.go")
65-
}, 0)
66-
if err != nil {
67-
return nil, err
58+
// An AST is a representation of the source code that can be traversed to extract information.
59+
// Converting files to AST representation to extract information.
60+
parseFiles, astFiles := []string{"clusterextension_types.go", "common_types.go"}, []*ast.File{}
61+
for _, file := range parseFiles {
62+
p, err := parser.ParseFile(fset, file, nil, 0)
63+
if err != nil {
64+
return nil, err
65+
}
66+
astFiles = append(astFiles, p)
6867
}
6968

7069
var constValues []string
7170

72-
// Iterate all of the top-level declarations in each package's files,
73-
// looking for constants that start with the prefix. When we find one,
74-
// add its value to the constValues list.
75-
for _, pkg := range pkgs {
76-
for _, f := range pkg.Files {
77-
for _, d := range f.Decls {
78-
genDecl, ok := d.(*ast.GenDecl)
79-
if !ok {
71+
// Iterate all of the top-level declarations in each file, looking
72+
// for constants that start with the prefix. When we find one, add
73+
// its value to the constValues list.
74+
for _, f := range astFiles {
75+
for _, d := range f.Decls {
76+
genDecl, ok := d.(*ast.GenDecl)
77+
if !ok {
78+
continue
79+
}
80+
for _, s := range genDecl.Specs {
81+
valueSpec, ok := s.(*ast.ValueSpec)
82+
if !ok || len(valueSpec.Names) != 1 || valueSpec.Names[0].Obj.Kind != ast.Con || !strings.HasPrefix(valueSpec.Names[0].String(), prefix) {
8083
continue
8184
}
82-
for _, s := range genDecl.Specs {
83-
valueSpec, ok := s.(*ast.ValueSpec)
84-
if !ok || len(valueSpec.Names) != 1 || valueSpec.Names[0].Obj.Kind != ast.Con || !strings.HasPrefix(valueSpec.Names[0].String(), prefix) {
85+
for _, val := range valueSpec.Values {
86+
lit, ok := val.(*ast.BasicLit)
87+
if !ok || lit.Kind != token.STRING {
8588
continue
8689
}
87-
for _, val := range valueSpec.Values {
88-
lit, ok := val.(*ast.BasicLit)
89-
if !ok || lit.Kind != token.STRING {
90-
continue
91-
}
92-
v, err := strconv.Unquote(lit.Value)
93-
if err != nil {
94-
return nil, fmt.Errorf("unquote literal string %s: %v", lit.Value, err)
95-
}
96-
constValues = append(constValues, v)
90+
v, err := strconv.Unquote(lit.Value)
91+
if err != nil {
92+
return nil, fmt.Errorf("unquote literal string %s: %v", lit.Value, err)
9793
}
94+
constValues = append(constValues, v)
9895
}
9996
}
10097
}

api/v1/common_types.go

-6
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ package v1
1919
const (
2020
TypeInstalled = "Installed"
2121
TypeProgressing = "Progressing"
22-
TypeServing = "Serving"
2322

2423
// Progressing reasons
2524
ReasonSucceeded = "Succeeded"
@@ -29,9 +28,4 @@ const (
2928
// Terminal reasons
3029
ReasonDeprecated = "Deprecated"
3130
ReasonFailed = "Failed"
32-
33-
// Serving reasons
34-
ReasonAvailable = "Available"
35-
ReasonUnavailable = "Unavailable"
36-
ReasonUserSpecifiedUnavailable = "UserSpecifiedUnavailable"
3731
)

internal/operator-controller/conditionsets/conditionsets.go

-4
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ var ConditionTypes = []string{
3131
ocv1.TypeChannelDeprecated,
3232
ocv1.TypeBundleDeprecated,
3333
ocv1.TypeProgressing,
34-
ocv1.TypeServing,
3534
}
3635

3736
var ConditionReasons = []string{
@@ -40,7 +39,4 @@ var ConditionReasons = []string{
4039
ocv1.ReasonFailed,
4140
ocv1.ReasonBlocked,
4241
ocv1.ReasonRetrying,
43-
ocv1.ReasonAvailable,
44-
ocv1.ReasonUnavailable,
45-
ocv1.ReasonUserSpecifiedUnavailable,
4642
}

0 commit comments

Comments
 (0)