Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions api/v1/clusterextension_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,17 @@ type ClusterExtensionSpec struct {
//
// +optional
Config *ClusterExtensionConfig `json:"config,omitempty"`

// progressDeadlineMinutes is an optional field that defines the maximum period
// of time in minutes after which an installation should be considered failed and
// require manual intervention. This functionality is disabled when no value
// is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours).
//
// +kubebuilder:validation:Minimum:=10
// +kubebuilder:validation:Maximum:=720
// +optional
// <opcon:experimental>
ProgressDeadlineMinutes int32 `json:"progressDeadlineMinutes,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add tests for this as well or do you feel it's unnecessary? Or if we could somehow run the same set of tests against the fields in both APIs it would be nice; particularly because we really want these to always match and that could serve as protection against one changing and not the other. As unlikely as it is that would actually happen 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, done.

}

const SourceTypeCatalog = "Catalog"
Expand Down
11 changes: 11 additions & 0 deletions api/v1/clusterextensionrevision_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,17 @@ type ClusterExtensionRevisionSpec struct {
// +listMapKey=name
// +optional
Phases []ClusterExtensionRevisionPhase `json:"phases,omitempty"`

// progressDeadlineMinutes is an optional field that defines the maximum period
// of time in minutes after which an installation should be considered failed and
// require manual intervention. This functionality is disabled when no value
// is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours).
//
// +kubebuilder:validation:Minimum:=10
// +kubebuilder:validation:Maximum:=720
// +optional
// <opcon:experimental>
ProgressDeadlineMinutes int32 `json:"progressDeadlineMinutes,omitempty"`
}

// ClusterExtensionRevisionLifecycleState specifies the lifecycle state of the ClusterExtensionRevision.
Expand Down
5 changes: 3 additions & 2 deletions api/v1/common_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const (
ReasonDeprecated = "Deprecated"

// Common reasons
ReasonSucceeded = "Succeeded"
ReasonFailed = "Failed"
ReasonSucceeded = "Succeeded"
ReasonFailed = "Failed"
ReasonProgressDeadlineExceeded = "ProgressDeadlineExceeded"
)
176 changes: 176 additions & 0 deletions api/v1/validation_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
package v1

import (
"fmt"
"testing"

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

func TestValidate(t *testing.T) {
type args struct {
object any
skipDefaulting bool
}
type want struct {
valid bool
}
type testCase struct {
args args
want want
}
defaultExtensionSpec := func(s *ClusterExtensionSpec) *ClusterExtensionSpec {
s.Namespace = "ns"
s.ServiceAccount = ServiceAccountReference{
Name: "sa",
}
s.Source = SourceConfig{
SourceType: SourceTypeCatalog,
Catalog: &CatalogFilter{
PackageName: "test",
},
}
return s
}
defaultRevisionSpec := func(s *ClusterExtensionRevisionSpec) *ClusterExtensionRevisionSpec {
s.Revision = 1
return s
}
c := newClient(t)
i := 0

for name, tc := range map[string]testCase{
"ClusterExtension: invalid progress deadline < 10": {
args: args{
object: ClusterExtensionSpec{
ProgressDeadlineMinutes: 9,
},
},
want: want{valid: false},
},
"ClusterExtension: valid progress deadline = 10": {
args: args{
object: ClusterExtensionSpec{
ProgressDeadlineMinutes: 10,
},
},
want: want{valid: true},
},
"ClusterExtension: valid progress deadline = 360": {
args: args{
object: ClusterExtensionSpec{
ProgressDeadlineMinutes: 360,
},
},
want: want{valid: true},
},
"ClusterExtension: valid progress deadline = 720": {
args: args{
object: ClusterExtensionSpec{
ProgressDeadlineMinutes: 720,
},
},
want: want{valid: true},
},
"ClusterExtension: invalid progress deadline > 720": {
args: args{
object: ClusterExtensionSpec{
ProgressDeadlineMinutes: 721,
},
},
want: want{valid: false},
},
"ClusterExtension: no progress deadline set": {
args: args{
object: ClusterExtensionSpec{},
},
want: want{valid: true},
},
"ClusterExtensionRevision: invalid progress deadline < 10": {
args: args{
object: ClusterExtensionRevisionSpec{
ProgressDeadlineMinutes: 9,
},
},
want: want{valid: false},
},
"ClusterExtensionRevision: valid progress deadline = 10": {
args: args{
object: ClusterExtensionRevisionSpec{
ProgressDeadlineMinutes: 10,
},
},
want: want{valid: true},
},
"ClusterExtensionRevision: valid progress deadline = 360": {
args: args{
object: ClusterExtensionRevisionSpec{
ProgressDeadlineMinutes: 360,
},
},
want: want{valid: true},
},
"ClusterExtensionRevision: valid progress deadline = 720": {
args: args{
object: ClusterExtensionRevisionSpec{
ProgressDeadlineMinutes: 720,
},
},
want: want{valid: true},
},
"ClusterExtensionRevision: invalid progress deadline > 720": {
args: args{
object: ClusterExtensionRevisionSpec{
ProgressDeadlineMinutes: 721,
},
},
want: want{valid: false},
},
"ClusterExtensionRevision: no progress deadline set": {
args: args{
object: ClusterExtensionRevisionSpec{},
},
want: want{valid: true},
},
} {
t.Run(name, func(t *testing.T) {
var obj client.Object
switch s := tc.args.object.(type) {
case ClusterExtensionSpec:
ce := &ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("ce-%d", i),
},
Spec: s,
}
if !tc.args.skipDefaulting {
defaultExtensionSpec(&ce.Spec)
}
obj = ce
case ClusterExtensionRevisionSpec:
cer := &ClusterExtensionRevision{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("cer-%d", i),
},
Spec: s,
}
if !tc.args.skipDefaulting {
defaultRevisionSpec(&cer.Spec)
}
obj = cer
default:
t.Fatalf("unknown type %T", s)
}
i++
err := c.Create(t.Context(), obj)
if tc.want.valid && err != nil {
t.Fatal("expected create to succeed, but got:", err)
}
if !tc.want.valid && !errors.IsInvalid(err) {
t.Fatal("expected create to fail due to invalid payload, but got:", err)
}
})
}
}
1 change: 1 addition & 0 deletions docs/api-reference/olmv1-api-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ _Appears in:_
| `source` _[SourceConfig](#sourceconfig)_ | source is required and selects the installation source of content for this ClusterExtension.<br />Set the sourceType field to perform the selection.<br />Catalog is currently the only implemented sourceType.<br />Setting sourceType to "Catalog" requires the catalog field to also be defined.<br />Below is a minimal example of a source definition (in yaml):<br />source:<br /> sourceType: Catalog<br /> catalog:<br /> packageName: example-package | | Required: \{\} <br /> |
| `install` _[ClusterExtensionInstallConfig](#clusterextensioninstallconfig)_ | install is optional and configures installation options for the ClusterExtension,<br />such as the pre-flight check configuration. | | |
| `config` _[ClusterExtensionConfig](#clusterextensionconfig)_ | config is optional and specifies bundle-specific configuration.<br />Configuration is bundle-specific and a bundle may provide a configuration schema.<br />When not specified, the default configuration of the resolved bundle is used.<br />config is validated against a configuration schema provided by the resolved bundle. If the bundle does not provide<br />a configuration schema the bundle is deemed to not be configurable. More information on how<br />to configure bundles can be found in the OLM documentation associated with your current OLM version. | | |
| `progressDeadlineMinutes` _integer_ | progressDeadlineMinutes is an optional field that defines the maximum period<br />of time in minutes after which an installation should be considered failed and<br />require manual intervention. This functionality is disabled when no value<br />is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours).<br /><opcon:experimental> | | Maximum: 720 <br />Minimum: 10 <br /> |


#### ClusterExtensionStatus
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,16 @@ spec:
x-kubernetes-validations:
- message: phases is immutable
rule: self == oldSelf || oldSelf.size() == 0
progressDeadlineMinutes:
description: |-
progressDeadlineMinutes is an optional field that defines the maximum period
of time in minutes after which an installation should be considered failed and
require manual intervention. This functionality is disabled when no value
is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours).
format: int32
maximum: 720
minimum: 10
type: integer
revision:
description: |-
revision is a required, immutable sequence number representing a specific revision
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,16 @@ spec:
rule: self == oldSelf
- message: namespace must be a valid DNS1123 label
rule: self.matches("^[a-z0-9]([-a-z0-9]*[a-z0-9])?$")
progressDeadlineMinutes:
description: |-
progressDeadlineMinutes is an optional field that defines the maximum period
of time in minutes after which an installation should be considered failed and
require manual intervention. This functionality is disabled when no value
is provided. The minimum period is 10 minutes, and the maximum is 720 minutes (12 hours).
format: int32
maximum: 720
minimum: 10
type: integer
serviceAccount:
description: |-
serviceAccount specifies a ServiceAccount used to perform all interactions with the cluster
Expand Down
6 changes: 5 additions & 1 deletion internal/operator-controller/applier/boxcutter.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision(
annotations[labels.ServiceAccountNameKey] = ext.Spec.ServiceAccount.Name
annotations[labels.ServiceAccountNamespaceKey] = ext.Spec.Namespace

return &ocv1.ClusterExtensionRevision{
cer := &ocv1.ClusterExtensionRevision{
ObjectMeta: metav1.ObjectMeta{
Annotations: annotations,
Labels: map[string]string{
Expand All @@ -206,6 +206,10 @@ func (r *SimpleRevisionGenerator) buildClusterExtensionRevision(
Phases: PhaseSort(objects),
},
}
if p := ext.Spec.ProgressDeadlineMinutes; p > 0 {
cer.Spec.ProgressDeadlineMinutes = p
}
return cer
}

// BoxcutterStorageMigrator migrates ClusterExtensions from Helm-based storage to
Expand Down
60 changes: 60 additions & 0 deletions internal/operator-controller/applier/boxcutter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/util/validation/field"
k8scheme "k8s.io/client-go/kubernetes/scheme"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
Expand Down Expand Up @@ -327,6 +328,65 @@ func Test_SimpleRevisionGenerator_AppliesObjectLabelsAndRevisionAnnotations(t *t
require.Equal(t, revAnnotations, rev.Annotations)
}

func Test_SimpleRevisionGenerator_PropagatesProgressDeadlineMinutes(t *testing.T) {
r := &FakeManifestProvider{
GetFn: func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) {
return []client.Object{}, nil
},
}

b := applier.SimpleRevisionGenerator{
Scheme: k8scheme.Scheme,
ManifestProvider: r,
}

type args struct {
progressDeadlineMinutes *int32
}
type want struct {
progressDeadlineMinutes int32
}
type testCase struct {
args args
want want
}
for name, tc := range map[string]testCase{
"propagates when set": {
args: args{
progressDeadlineMinutes: ptr.To(int32(10)),
},
want: want{
progressDeadlineMinutes: 10,
},
},
"do not propagate when unset": {
want: want{
progressDeadlineMinutes: 0,
},
},
} {
ext := &ocv1.ClusterExtension{
ObjectMeta: metav1.ObjectMeta{
Name: "test-extension",
},
Spec: ocv1.ClusterExtensionSpec{
Namespace: "test-namespace",
ServiceAccount: ocv1.ServiceAccountReference{Name: "test-sa"},
},
}
empty := map[string]string{}
t.Run(name, func(t *testing.T) {
if pd := tc.args.progressDeadlineMinutes; pd != nil {
ext.Spec.ProgressDeadlineMinutes = *pd
}

rev, err := b.GenerateRevision(t.Context(), fstest.MapFS{}, ext, empty, empty)
require.NoError(t, err)
require.Equal(t, tc.want.progressDeadlineMinutes, rev.Spec.ProgressDeadlineMinutes)
})
}
}

func Test_SimpleRevisionGenerator_Failure(t *testing.T) {
r := &FakeManifestProvider{
GetFn: func(b fs.FS, e *ocv1.ClusterExtension) ([]client.Object, error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,5 @@ var ConditionReasons = []string{
ocv1.ReasonRetrying,
ocv1.ReasonAbsent,
ocv1.ReasonRollingOut,
ocv1.ReasonProgressDeadlineExceeded,
}
Loading
Loading