Skip to content

Commit

Permalink
Inline the ready condition builder in the patching reconciler
Browse files Browse the repository at this point in the history
* introduce `k8s.RuntimeObjectWithStatusConditions`
* replace `conditions.RuntimeObjectWithStatusConditions` with
  `k8s.RuntimeObjectWithStatusConditions` - that made the generic
  signature of the awaiter a bit awkward
* Make the ready condition builder availble into the patching reconciler
  and remove it from all the controllers
* introduce the `k8s.NotReadyError` error. By returning that error in
  controllers, the patching reconciler sets the `Ready` condition
  accordingly
* The kpack build reconciler no longer uses the patching reconciler as
  kpack build custom resource does not implement the
  `RuntimeObjectWithStatusConditions` interface

Co-authored-by: Georgi Sabev <[email protected]>
  • Loading branch information
danail-branekov and georgethebeatle committed Aug 30, 2024
1 parent 0b9f256 commit 790847c
Show file tree
Hide file tree
Showing 47 changed files with 381 additions and 234 deletions.
14 changes: 7 additions & 7 deletions api/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,14 +128,14 @@ func main() {
privilegedCRClient,
userClientFactory,
nsPermissions,
conditions.NewConditionAwaiter[*korifiv1alpha1.CFOrg, korifiv1alpha1.CFOrgList](conditionTimeout),
conditions.NewConditionAwaiter[*korifiv1alpha1.CFOrg, korifiv1alpha1.CFOrg, korifiv1alpha1.CFOrgList](conditionTimeout),
)
spaceRepo := repositories.NewSpaceRepo(
namespaceRetriever,
orgRepo,
userClientFactory,
nsPermissions,
conditions.NewConditionAwaiter[*korifiv1alpha1.CFSpace, korifiv1alpha1.CFSpaceList](conditionTimeout),
conditions.NewConditionAwaiter[*korifiv1alpha1.CFSpace, korifiv1alpha1.CFSpace, korifiv1alpha1.CFSpaceList](conditionTimeout),
)
processRepo := repositories.NewProcessRepo(
namespaceRetriever,
Expand All @@ -149,7 +149,7 @@ func main() {
namespaceRetriever,
userClientFactory,
nsPermissions,
conditions.NewConditionAwaiter[*korifiv1alpha1.CFApp, korifiv1alpha1.CFAppList](conditionTimeout),
conditions.NewConditionAwaiter[*korifiv1alpha1.CFApp, korifiv1alpha1.CFApp, korifiv1alpha1.CFAppList](conditionTimeout),
)
dropletRepo := repositories.NewDropletRepo(
userClientFactory,
Expand Down Expand Up @@ -189,19 +189,19 @@ func main() {
nsPermissions,
toolsregistry.NewRepositoryCreator(cfg.ContainerRegistryType),
cfg.ContainerRepositoryPrefix,
conditions.NewConditionAwaiter[*korifiv1alpha1.CFPackage, korifiv1alpha1.CFPackageList](conditionTimeout),
conditions.NewConditionAwaiter[*korifiv1alpha1.CFPackage, korifiv1alpha1.CFPackage, korifiv1alpha1.CFPackageList](conditionTimeout),
)
serviceInstanceRepo := repositories.NewServiceInstanceRepo(
namespaceRetriever,
userClientFactory,
nsPermissions,
conditions.NewConditionAwaiter[*korifiv1alpha1.CFServiceInstance, korifiv1alpha1.CFServiceInstanceList](conditionTimeout),
conditions.NewConditionAwaiter[*korifiv1alpha1.CFServiceInstance, korifiv1alpha1.CFServiceInstance, korifiv1alpha1.CFServiceInstanceList](conditionTimeout),
)
serviceBindingRepo := repositories.NewServiceBindingRepo(
namespaceRetriever,
userClientFactory,
nsPermissions,
conditions.NewConditionAwaiter[*korifiv1alpha1.CFServiceBinding, korifiv1alpha1.CFServiceBindingList](conditionTimeout),
conditions.NewConditionAwaiter[*korifiv1alpha1.CFServiceBinding, korifiv1alpha1.CFServiceBinding, korifiv1alpha1.CFServiceBindingList](conditionTimeout),
)
buildpackRepo := repositories.NewBuildpackRepository(cfg.BuilderName,
userClientFactory,
Expand All @@ -228,7 +228,7 @@ func main() {
userClientFactory,
namespaceRetriever,
nsPermissions,
conditions.NewConditionAwaiter[*korifiv1alpha1.CFTask, korifiv1alpha1.CFTaskList](conditionTimeout),
conditions.NewConditionAwaiter[*korifiv1alpha1.CFTask, korifiv1alpha1.CFTask, korifiv1alpha1.CFTaskList](conditionTimeout),
)
metricsRepo := repositories.NewMetricsRepo(userClientFactory)
serviceBrokerRepo := repositories.NewServiceBrokerRepo(userClientFactory, cfg.RootNamespace)
Expand Down
2 changes: 2 additions & 0 deletions api/repositories/app_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ var _ = Describe("AppRepository", func() {
var (
appAwaiter *fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFApp,
korifiv1alpha1.CFApp,
korifiv1alpha1.CFAppList,
*korifiv1alpha1.CFAppList,
]
Expand All @@ -50,6 +51,7 @@ var _ = Describe("AppRepository", func() {
BeforeEach(func() {
appAwaiter = &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFApp,
korifiv1alpha1.CFApp,
korifiv1alpha1.CFAppList,
*korifiv1alpha1.CFAppList,
]{}
Expand Down
20 changes: 8 additions & 12 deletions api/repositories/conditions/await.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,38 +5,34 @@ import (
"fmt"
"time"

"code.cloudfoundry.org/korifi/tools/k8s"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
)

type RuntimeObjectWithStatusConditions interface {
client.Object
StatusConditions() []metav1.Condition
}

type ObjectList[L any] interface {
*L
client.ObjectList
}

type Awaiter[T RuntimeObjectWithStatusConditions, L any, PL ObjectList[L]] struct {
type Awaiter[T k8s.RuntimeObjectWithStatusConditions[TT], TT any, L any, PL ObjectList[L]] struct {
timeout time.Duration
}

func NewConditionAwaiter[T RuntimeObjectWithStatusConditions, L any, PL ObjectList[L]](timeout time.Duration) *Awaiter[T, L, PL] {
return &Awaiter[T, L, PL]{
func NewConditionAwaiter[T k8s.RuntimeObjectWithStatusConditions[TT], TT any, L any, PL ObjectList[L]](timeout time.Duration) *Awaiter[T, TT, L, PL] {
return &Awaiter[T, TT, L, PL]{
timeout: timeout,
}
}

func (a *Awaiter[T, L, PL]) AwaitCondition(ctx context.Context, k8sClient client.WithWatch, object client.Object, conditionType string) (T, error) {
func (a *Awaiter[T, TT, L, PL]) AwaitCondition(ctx context.Context, k8sClient client.WithWatch, object client.Object, conditionType string) (T, error) {
return a.AwaitState(ctx, k8sClient, object, func(o T) error {
return checkConditionIsTrue(o, conditionType)
})
}

func (a *Awaiter[T, L, PL]) AwaitState(ctx context.Context, k8sClient client.WithWatch, object client.Object, checkState func(T) error) (T, error) {
func (a *Awaiter[T, TT, L, PL]) AwaitState(ctx context.Context, k8sClient client.WithWatch, object client.Object, checkState func(T) error) (T, error) {
var empty T
objList := PL(new(L))

Expand Down Expand Up @@ -71,8 +67,8 @@ func (a *Awaiter[T, L, PL]) AwaitState(ctx context.Context, k8sClient client.Wit
)
}

func checkConditionIsTrue[T RuntimeObjectWithStatusConditions](obj T, conditionType string) error {
condition := meta.FindStatusCondition(obj.StatusConditions(), conditionType)
func checkConditionIsTrue[T k8s.RuntimeObjectWithStatusConditions[L], L any](obj T, conditionType string) error {
condition := meta.FindStatusCondition(*obj.StatusConditions(), conditionType)

if condition == nil {
return fmt.Errorf("condition %s not set yet", conditionType)
Expand Down
4 changes: 2 additions & 2 deletions api/repositories/conditions/await_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import (

var _ = Describe("ConditionAwaiter", func() {
var (
awaiter *conditions.Awaiter[*korifiv1alpha1.CFTask, korifiv1alpha1.CFTaskList, *korifiv1alpha1.CFTaskList]
awaiter *conditions.Awaiter[*korifiv1alpha1.CFTask, korifiv1alpha1.CFTask, korifiv1alpha1.CFTaskList, *korifiv1alpha1.CFTaskList]
task *korifiv1alpha1.CFTask
awaitedTask *korifiv1alpha1.CFTask
awaitErr error
Expand All @@ -43,7 +43,7 @@ var _ = Describe("ConditionAwaiter", func() {
}

BeforeEach(func() {
awaiter = conditions.NewConditionAwaiter[*korifiv1alpha1.CFTask, korifiv1alpha1.CFTaskList](time.Second)
awaiter = conditions.NewConditionAwaiter[*korifiv1alpha1.CFTask, korifiv1alpha1.CFTask, korifiv1alpha1.CFTaskList](time.Second)
awaitedTask = nil
awaitErr = nil

Expand Down
19 changes: 10 additions & 9 deletions api/repositories/fakeawaiter/await.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import (
"context"

"code.cloudfoundry.org/korifi/api/repositories/conditions"
"code.cloudfoundry.org/korifi/tools/k8s"
"sigs.k8s.io/controller-runtime/pkg/client"
)

type FakeAwaiter[T conditions.RuntimeObjectWithStatusConditions, L any, PL conditions.ObjectList[L]] struct {
type FakeAwaiter[T k8s.RuntimeObjectWithStatusConditions[TT], TT any, L any, PL conditions.ObjectList[L]] struct {
awaitConditionCalls []struct {
obj client.Object
conditionType string
Expand All @@ -20,7 +21,7 @@ type FakeAwaiter[T conditions.RuntimeObjectWithStatusConditions, L any, PL condi
}
}

func (a *FakeAwaiter[T, L, PL]) AwaitCondition(ctx context.Context, k8sClient client.WithWatch, object client.Object, conditionType string) (T, error) {
func (a *FakeAwaiter[T, TT, L, PL]) AwaitCondition(ctx context.Context, k8sClient client.WithWatch, object client.Object, conditionType string) (T, error) {
a.awaitConditionCalls = append(a.awaitConditionCalls, struct {
obj client.Object
conditionType string
Expand All @@ -36,21 +37,21 @@ func (a *FakeAwaiter[T, L, PL]) AwaitCondition(ctx context.Context, k8sClient cl
return a.AwaitConditionStub(ctx, k8sClient, object, conditionType)
}

func (a *FakeAwaiter[T, L, PL]) AwaitConditionReturns(object T, err error) {
func (a *FakeAwaiter[T, TT, L, PL]) AwaitConditionReturns(object T, err error) {
a.AwaitConditionStub = func(ctx context.Context, k8sClient client.WithWatch, object client.Object, conditionType string) (T, error) {
return object.(T), err
}
}

func (a *FakeAwaiter[T, L, PL]) AwaitConditionCallCount() int {
func (a *FakeAwaiter[T, TT, L, PL]) AwaitConditionCallCount() int {
return len(a.awaitConditionCalls)
}

func (a *FakeAwaiter[T, L, PL]) AwaitConditionArgsForCall(i int) (client.Object, string) {
func (a *FakeAwaiter[T, TT, L, PL]) AwaitConditionArgsForCall(i int) (client.Object, string) {
return a.awaitConditionCalls[i].obj, a.awaitConditionCalls[i].conditionType
}

func (a *FakeAwaiter[T, L, PL]) AwaitState(ctx context.Context, k8sClient client.WithWatch, object client.Object, checkState func(T) error) (T, error) {
func (a *FakeAwaiter[T, TT, L, PL]) AwaitState(ctx context.Context, k8sClient client.WithWatch, object client.Object, checkState func(T) error) (T, error) {
a.awaitStateCalls = append(a.awaitStateCalls, struct {
obj client.Object
checkState func(T) error
Expand All @@ -66,16 +67,16 @@ func (a *FakeAwaiter[T, L, PL]) AwaitState(ctx context.Context, k8sClient client
return a.AwaitStateStub(ctx, k8sClient, object, checkState)
}

func (a *FakeAwaiter[T, L, PL]) AwaitStateReturns(object T, err error) {
func (a *FakeAwaiter[T, TT, L, PL]) AwaitStateReturns(object T, err error) {
a.AwaitStateStub = func(ctx context.Context, k8sClient client.WithWatch, object client.Object, _ func(T) error) (T, error) {
return object.(T), err
}
}

func (a *FakeAwaiter[T, L, PL]) AwaitStateCallCount() int {
func (a *FakeAwaiter[T, TT, L, PL]) AwaitStateCallCount() int {
return len(a.awaitStateCalls)
}

func (a *FakeAwaiter[T, L, PL]) AwaitStateArgsForCall(i int) (client.Object, func(T) error) {
func (a *FakeAwaiter[T, TT, L, PL]) AwaitStateArgsForCall(i int) (client.Object, func(T) error) {
return a.awaitStateCalls[i].obj, a.awaitStateCalls[i].checkState
}
2 changes: 2 additions & 0 deletions api/repositories/org_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var _ = Describe("OrgRepository", func() {
var (
conditionAwaiter *fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFOrg,
korifiv1alpha1.CFOrg,
korifiv1alpha1.CFOrgList,
*korifiv1alpha1.CFOrgList,
]
Expand All @@ -36,6 +37,7 @@ var _ = Describe("OrgRepository", func() {
BeforeEach(func() {
conditionAwaiter = &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFOrg,
korifiv1alpha1.CFOrg,
korifiv1alpha1.CFOrgList,
*korifiv1alpha1.CFOrgList,
]{}
Expand Down
2 changes: 2 additions & 0 deletions api/repositories/package_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ var _ = Describe("PackageRepository", func() {
repoCreator *fake.RepositoryCreator
conditionAwaiter *fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFPackage,
korifiv1alpha1.CFPackage,
korifiv1alpha1.CFPackageList,
*korifiv1alpha1.CFPackageList,
]
Expand All @@ -45,6 +46,7 @@ var _ = Describe("PackageRepository", func() {
repoCreator = new(fake.RepositoryCreator)
conditionAwaiter = &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFPackage,
korifiv1alpha1.CFPackage,
korifiv1alpha1.CFPackageList,
*korifiv1alpha1.CFPackageList,
]{}
Expand Down
2 changes: 2 additions & 0 deletions api/repositories/role_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,13 @@ var _ = Describe("RoleRepository", func() {
}
orgRepo := repositories.NewOrgRepo(rootNamespace, k8sClient, userClientFactory, nsPerms, &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFOrg,
korifiv1alpha1.CFOrg,
korifiv1alpha1.CFOrgList,
*korifiv1alpha1.CFOrgList,
]{})
spaceRepo := repositories.NewSpaceRepo(namespaceRetriever, orgRepo, userClientFactory, nsPerms, &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFSpace,
korifiv1alpha1.CFSpace,
korifiv1alpha1.CFSpaceList,
*korifiv1alpha1.CFSpaceList,
]{})
Expand Down
2 changes: 2 additions & 0 deletions api/repositories/service_binding_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ var _ = Describe("ServiceBindingRepo", func() {
bindingName *string
conditionAwaiter *fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFServiceBinding,
korifiv1alpha1.CFServiceBinding,
korifiv1alpha1.CFServiceBindingList,
*korifiv1alpha1.CFServiceBindingList,
]
Expand All @@ -42,6 +43,7 @@ var _ = Describe("ServiceBindingRepo", func() {
testCtx = context.Background()
conditionAwaiter = &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFServiceBinding,
korifiv1alpha1.CFServiceBinding,
korifiv1alpha1.CFServiceBindingList,
*korifiv1alpha1.CFServiceBindingList,
]{}
Expand Down
2 changes: 2 additions & 0 deletions api/repositories/service_instance_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ var _ = Describe("ServiceInstanceRepository", func() {
serviceInstanceRepo *repositories.ServiceInstanceRepo
conditionAwaiter *fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFServiceInstance,
korifiv1alpha1.CFServiceInstance,
korifiv1alpha1.CFServiceInstanceList,
*korifiv1alpha1.CFServiceInstanceList,
]
Expand All @@ -46,6 +47,7 @@ var _ = Describe("ServiceInstanceRepository", func() {
testCtx = context.Background()
conditionAwaiter = &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFServiceInstance,
korifiv1alpha1.CFServiceInstance,
korifiv1alpha1.CFServiceInstanceList,
*korifiv1alpha1.CFServiceInstanceList,
]{}
Expand Down
1 change: 1 addition & 0 deletions api/repositories/service_plan_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var _ = Describe("ServicePlanRepo", func() {
BeforeEach(func() {
orgRepo := repositories.NewOrgRepo(rootNamespace, k8sClient, userClientFactory, nsPerms, &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFOrg,
korifiv1alpha1.CFOrg,
korifiv1alpha1.CFOrgList,
*korifiv1alpha1.CFOrgList,
]{})
Expand Down
3 changes: 3 additions & 0 deletions api/repositories/space_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ var _ = Describe("SpaceRepository", func() {
orgRepo *repositories.OrgRepo
conditionAwaiter *fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFSpace,
korifiv1alpha1.CFSpace,
korifiv1alpha1.CFSpaceList,
*korifiv1alpha1.CFSpaceList,
]
Expand All @@ -36,12 +37,14 @@ var _ = Describe("SpaceRepository", func() {
BeforeEach(func() {
orgRepo = repositories.NewOrgRepo(rootNamespace, k8sClient, userClientFactory, nsPerms, &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFOrg,
korifiv1alpha1.CFOrg,
korifiv1alpha1.CFOrgList,
*korifiv1alpha1.CFOrgList,
]{})

conditionAwaiter = &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFSpace,
korifiv1alpha1.CFSpace,
korifiv1alpha1.CFSpaceList,
*korifiv1alpha1.CFSpaceList,
]{}
Expand Down
2 changes: 2 additions & 0 deletions api/repositories/task_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ var _ = Describe("TaskRepository", func() {
var (
conditionAwaiter *fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFTask,
korifiv1alpha1.CFTask,
korifiv1alpha1.CFTaskList,
*korifiv1alpha1.CFTaskList,
]
Expand All @@ -40,6 +41,7 @@ var _ = Describe("TaskRepository", func() {
BeforeEach(func() {
conditionAwaiter = &fakeawaiter.FakeAwaiter[
*korifiv1alpha1.CFTask,
korifiv1alpha1.CFTask,
korifiv1alpha1.CFTaskList,
*korifiv1alpha1.CFTaskList,
]{}
Expand Down
4 changes: 4 additions & 0 deletions controllers/api/v1alpha1/appworkload_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ type AppWorkload struct {
Status AppWorkloadStatus `json:"status,omitempty"`
}

func (w *AppWorkload) StatusConditions() *[]metav1.Condition {
return &w.Status.Conditions
}

//+kubebuilder:object:root=true

// AppWorkloadList contains a list of AppWorkload
Expand Down
4 changes: 4 additions & 0 deletions controllers/api/v1alpha1/builderinfo_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ type BuilderInfo struct {
Status BuilderInfoStatus `json:"status,omitempty"`
}

func (i *BuilderInfo) StatusConditions() *[]metav1.Condition {
return &i.Status.Conditions
}

//+kubebuilder:object:root=true

// BuilderInfoList contains a list of BuilderInfo
Expand Down
4 changes: 4 additions & 0 deletions controllers/api/v1alpha1/buildworkload_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ type BuildWorkload struct {
Status BuildWorkloadStatus `json:"status,omitempty"`
}

func (w *BuildWorkload) StatusConditions() *[]metav1.Condition {
return &w.Status.Conditions
}

//+kubebuilder:object:root=true

// BuildWorkloadList contains a list of BuildWorkload
Expand Down
4 changes: 2 additions & 2 deletions controllers/api/v1alpha1/cfapp_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ func init() {
SchemeBuilder.Register(&CFApp{}, &CFAppList{})
}

func (a CFApp) StatusConditions() []metav1.Condition {
return a.Status.Conditions
func (a *CFApp) StatusConditions() *[]metav1.Condition {
return &a.Status.Conditions
}

func (a CFApp) UniqueName() string {
Expand Down
4 changes: 4 additions & 0 deletions controllers/api/v1alpha1/cfbuild_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,10 @@ type CFBuild struct {
Status CFBuildStatus `json:"status,omitempty"`
}

func (b *CFBuild) StatusConditions() *[]metav1.Condition {
return &b.Status.Conditions
}

//+kubebuilder:object:root=true

// CFBuildList contains a list of CFBuild
Expand Down
Loading

0 comments on commit 790847c

Please sign in to comment.