From 3edcd16b62ba509f92eda89d69b72f6e78bc9179 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 23 Mar 2022 12:15:31 +0100 Subject: [PATCH] fix: wait for Source objects observed generation This ensures the command will wait for the object to report a Ready Condition with an ObservedGeneration matching the Generation of the resource. Ensuring that when a "create" is actually a mutation, it waits instead of prematurely assuming the Source to be Ready. Signed-off-by: Hidde Beydals --- cmd/flux/create_source_bucket.go | 29 ++++++++++++++++++++++++++ cmd/flux/create_source_git.go | 17 +++++++++++---- cmd/flux/create_source_git_test.go | 27 +++++++++++++----------- cmd/flux/create_source_helm.go | 14 +++++++------ cmd/flux/reconcile_source_bucket.go | 32 ----------------------------- go.mod | 1 + go.sum | 11 ++++++++++ 7 files changed, 77 insertions(+), 54 deletions(-) diff --git a/cmd/flux/create_source_bucket.go b/cmd/flux/create_source_bucket.go index 89616451..6160be0c 100644 --- a/cmd/flux/create_source_bucket.go +++ b/cmd/flux/create_source_bucket.go @@ -30,6 +30,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/fluxcd/flux2/internal/flags" @@ -235,3 +237,30 @@ func upsertBucket(ctx context.Context, kubeClient client.Client, logger.Successf("Bucket source updated") return namespacedName, nil } + +func isBucketReady(ctx context.Context, kubeClient client.Client, + namespacedName types.NamespacedName, bucket *sourcev1.Bucket) wait.ConditionFunc { + return func() (bool, error) { + err := kubeClient.Get(ctx, namespacedName, bucket) + if err != nil { + return false, err + } + + if c := conditions.Get(bucket, meta.ReadyCondition); c != nil { + // Confirm the Ready condition we are observing is for the + // current generation + if c.ObservedGeneration != bucket.GetGeneration() { + return false, nil + } + + // Further check the Status + switch c.Status { + case metav1.ConditionTrue: + return true, nil + case metav1.ConditionFalse: + return false, fmt.Errorf(c.Message) + } + } + return false, nil + } +} diff --git a/cmd/flux/create_source_git.go b/cmd/flux/create_source_git.go index 7e730fe2..28c1be50 100644 --- a/cmd/flux/create_source_git.go +++ b/cmd/flux/create_source_git.go @@ -23,19 +23,21 @@ import ( "net/url" "os" - "github.com/fluxcd/pkg/apis/meta" - sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" "github.com/manifoldco/promptui" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/yaml" + "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" + + sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" + "github.com/fluxcd/flux2/internal/flags" "github.com/fluxcd/flux2/internal/utils" "github.com/fluxcd/flux2/pkg/manifestgen/sourcesecret" @@ -355,7 +357,14 @@ func isGitRepositoryReady(ctx context.Context, kubeClient client.Client, return false, err } - if c := apimeta.FindStatusCondition(gitRepository.Status.Conditions, meta.ReadyCondition); c != nil { + if c := conditions.Get(gitRepository, meta.ReadyCondition); c != nil { + // Confirm the Ready condition we are observing is for the + // current generation + if c.ObservedGeneration != gitRepository.GetGeneration() { + return false, nil + } + + // Further check the Status switch c.Status { case metav1.ConditionTrue: return true, nil diff --git a/cmd/flux/create_source_git_test.go b/cmd/flux/create_source_git_test.go index b6b9961f..6447e52e 100644 --- a/cmd/flux/create_source_git_test.go +++ b/cmd/flux/create_source_git_test.go @@ -106,10 +106,11 @@ func TestCreateSourceGit(t *testing.T) { assertGoldenFile("testdata/create_source_git/success.golden"), func(repo *sourcev1.GitRepository) { newCondition := metav1.Condition{ - Type: meta.ReadyCondition, - Status: metav1.ConditionTrue, - Reason: sourcev1.GitOperationSucceedReason, - Message: "succeeded message", + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + Reason: sourcev1.GitOperationSucceedReason, + Message: "succeeded message", + ObservedGeneration: repo.GetGeneration(), } apimeta.SetStatusCondition(&repo.Status.Conditions, newCondition) repo.Status.Artifact = &sourcev1.Artifact{ @@ -123,10 +124,11 @@ func TestCreateSourceGit(t *testing.T) { assertError("failed message"), func(repo *sourcev1.GitRepository) { newCondition := metav1.Condition{ - Type: meta.ReadyCondition, - Status: metav1.ConditionFalse, - Reason: sourcev1.URLInvalidReason, - Message: "failed message", + Type: meta.ReadyCondition, + Status: metav1.ConditionFalse, + Reason: sourcev1.URLInvalidReason, + Message: "failed message", + ObservedGeneration: repo.GetGeneration(), } apimeta.SetStatusCondition(&repo.Status.Conditions, newCondition) }, @@ -137,10 +139,11 @@ func TestCreateSourceGit(t *testing.T) { func(repo *sourcev1.GitRepository) { // Updated with no artifact newCondition := metav1.Condition{ - Type: meta.ReadyCondition, - Status: metav1.ConditionTrue, - Reason: sourcev1.GitOperationSucceedReason, - Message: "succeeded message", + Type: meta.ReadyCondition, + Status: metav1.ConditionTrue, + Reason: sourcev1.GitOperationSucceedReason, + Message: "succeeded message", + ObservedGeneration: repo.GetGeneration(), } apimeta.SetStatusCondition(&repo.Status.Conditions, newCondition) }, diff --git a/cmd/flux/create_source_helm.go b/cmd/flux/create_source_helm.go index f20b4658..789ff6e5 100644 --- a/cmd/flux/create_source_helm.go +++ b/cmd/flux/create_source_helm.go @@ -23,10 +23,10 @@ import ( "os" "github.com/fluxcd/pkg/apis/meta" + "github.com/fluxcd/pkg/runtime/conditions" "github.com/spf13/cobra" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" - apimeta "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/wait" @@ -242,12 +242,14 @@ func isHelmRepositoryReady(ctx context.Context, kubeClient client.Client, return false, err } - // Confirm the state we are observing is for the current generation - if helmRepository.Generation != helmRepository.Status.ObservedGeneration { - return false, nil - } + if c := conditions.Get(helmRepository, meta.ReadyCondition); c != nil { + // Confirm the Ready condition we are observing is for the + // current generation + if c.ObservedGeneration != helmRepository.GetGeneration() { + return false, nil + } - if c := apimeta.FindStatusCondition(helmRepository.Status.Conditions, meta.ReadyCondition); c != nil { + // Further check the Status switch c.Status { case metav1.ConditionTrue: return true, nil diff --git a/cmd/flux/reconcile_source_bucket.go b/cmd/flux/reconcile_source_bucket.go index 3ede1c50..2a2bbdbc 100644 --- a/cmd/flux/reconcile_source_bucket.go +++ b/cmd/flux/reconcile_source_bucket.go @@ -17,17 +17,10 @@ limitations under the License. package main import ( - "context" "fmt" "github.com/spf13/cobra" - apimeta "k8s.io/apimachinery/pkg/api/meta" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - "k8s.io/apimachinery/pkg/types" - "k8s.io/apimachinery/pkg/util/wait" - "sigs.k8s.io/controller-runtime/pkg/client" - "github.com/fluxcd/pkg/apis/meta" sourcev1 "github.com/fluxcd/source-controller/api/v1beta2" ) @@ -48,31 +41,6 @@ func init() { reconcileSourceCmd.AddCommand(reconcileSourceBucketCmd) } -func isBucketReady(ctx context.Context, kubeClient client.Client, - namespacedName types.NamespacedName, bucket *sourcev1.Bucket) wait.ConditionFunc { - return func() (bool, error) { - err := kubeClient.Get(ctx, namespacedName, bucket) - if err != nil { - return false, err - } - - // Confirm the state we are observing is for the current generation - if bucket.Generation != bucket.Status.ObservedGeneration { - return false, nil - } - - if c := apimeta.FindStatusCondition(bucket.Status.Conditions, meta.ReadyCondition); c != nil { - switch c.Status { - case metav1.ConditionTrue: - return true, nil - case metav1.ConditionFalse: - return false, fmt.Errorf(c.Message) - } - } - return false, nil - } -} - func (obj bucketAdapter) lastHandledReconcileRequest() string { return obj.Status.GetLastHandledReconcileRequest() } diff --git a/go.mod b/go.mod index 17ac2bbe..bdf9bc9b 100644 --- a/go.mod +++ b/go.mod @@ -122,6 +122,7 @@ require ( github.com/modern-go/concurrent v0.0.0-20180306012644-bacd9c7ef1dd // indirect github.com/modern-go/reflect2 v1.0.2 // indirect github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00 // indirect + github.com/onsi/gomega v1.18.1 // indirect github.com/peterbourgon/diskv v2.0.1+incompatible // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect diff --git a/go.sum b/go.sum index e08705dd..d9bee2db 100644 --- a/go.sum +++ b/go.sum @@ -466,6 +466,7 @@ github.com/go-openapi/swag v0.19.5/go.mod h1:POnQmlKehdgb5mhVOsnJFsivZCEZ/vjK9gh github.com/go-openapi/swag v0.19.14 h1:gm3vOOXfiuw5i9p5N9xJvfjvuofpyvLA9Wr6QfK5Fng= github.com/go-openapi/swag v0.19.14/go.mod h1:QYRuS/SOXUCsnplDa677K7+DxSOj6IPNl/eQntq43wQ= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= +github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE= github.com/godbus/dbus v0.0.0-20151105175453-c7fdd8b5cd55/go.mod h1:/YcGZj5zSblfDWMMoOzV4fas9FZnQYTkDnsGvmh2Grw= github.com/godbus/dbus v0.0.0-20180201030542-885f9cc04c9c/go.mod h1:/YcGZj5zSblfDWMMoOzV4fas9FZnQYTkDnsGvmh2Grw= github.com/godbus/dbus v0.0.0-20190422162347-ade71ed3457e/go.mod h1:bBOAhwG1umN6/6ZUMtDFBMQR8jRg9O75tm9K00oMsK4= @@ -575,6 +576,7 @@ github.com/google/pprof v0.0.0-20201023163331-3e6fc7fc9c4c/go.mod h1:kpwsk12EmLe github.com/google/pprof v0.0.0-20201203190320-1bf35d6f28c2/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/google/pprof v0.0.0-20210122040257-d980be63207e/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/google/pprof v0.0.0-20210226084205-cbba55b83ad5/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= +github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/google/pprof v0.0.0-20210601050228-01bbb1931b22/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/google/pprof v0.0.0-20210609004039-a478d1d731e9/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= @@ -820,6 +822,7 @@ github.com/ncw/swift v1.0.47/go.mod h1:23YIA4yWVnGwv2dQlN4bB7egfYX6YLn0Yo/S6zZO/ github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno= github.com/nxadm/tail v1.4.4/go.mod h1:kenIhsEOeOJmVchQTgglprH7qJGnHDVpk1VPCcaMI8A= github.com/nxadm/tail v1.4.8 h1:nPr65rt6Y5JFSKQO7qToXr7pePgD6Gwiw05lkbyAQTE= +github.com/nxadm/tail v1.4.8/go.mod h1:+ncqLTQzXmGhMZNUePPaPqPvBxHAIsmXswZKocGu+AU= github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= github.com/olekukonko/tablewriter v0.0.0-20170122224234-a0225b3f23b5/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo= github.com/olekukonko/tablewriter v0.0.4/go.mod h1:zq6QwlOf5SlnkVbMSr5EoBv3636FWnp+qbPhuoO21uA= @@ -833,14 +836,19 @@ github.com/onsi/ginkgo v1.10.3/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+ github.com/onsi/ginkgo v1.11.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE= github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk= github.com/onsi/ginkgo v1.14.0/go.mod h1:iSB4RoI2tjJc9BBv4NKIKWKya62Rps+oPG/Lv9klQyY= +github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0= github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= +github.com/onsi/ginkgo/v2 v2.0.0 h1:CcuG/HvWNkkaqCUpJifQY8z7qEMBJya6aLPx6ftGyjQ= +github.com/onsi/ginkgo/v2 v2.0.0/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c= github.com/onsi/gomega v0.0.0-20151007035656-2152b45fa28a/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/onsi/gomega v1.10.3/go.mod h1:V9xEwhxec5O8UDM77eCW8vLymOMltsqPVYWrpDsH8xc= +github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= github.com/onsi/gomega v1.18.1 h1:M1GfJqGRrBrrGGsbxzV5dqM2U2ApXefZCQpkukxYRLE= +github.com/onsi/gomega v1.18.1/go.mod h1:0q+aL8jAiMXy9hbwj2mr5GziHiwhAIQpFmmtT5hitRs= github.com/opencontainers/go-digest v0.0.0-20170106003457-a6d0ee40d420/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/go-digest v0.0.0-20180430190053-c9281466c8b2/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= github.com/opencontainers/go-digest v1.0.0-rc1/go.mod h1:cMLVZDEM3+U2I4VmLI6N8jQYUd2OVphdqWwCJHrFt2s= @@ -1211,6 +1219,7 @@ golang.org/x/net v0.0.0-20210316092652-d523dce5a7f4/go.mod h1:RBQZq4jEuRlivfhVLd golang.org/x/net v0.0.0-20210326060303-6b1517762897/go.mod h1:uSPa2vr4CLtc/ILN5odXGNXS6mhrKVzTaCXzk9m6W3k= golang.org/x/net v0.0.0-20210405180319-a5a99cb37ef4/go.mod h1:p54w0d4576C0XHj96bSt6lcn1PtDYWL6XObtHCRCNQM= golang.org/x/net v0.0.0-20210410081132-afb366fc7cd1/go.mod h1:9tjilg8BloeKEkVJvy7fQ90B1CfIiPueXVOjqfkSzI8= +golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk= golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20210525063256-abc453219eb5/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20210805182204-aaa1db679c0d/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= @@ -1330,6 +1339,7 @@ golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20201201145000-ef89a241ccb3/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20201202213521-69691e467435/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210104204734-6f8348627aad/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210119212857-b64e53b001e4/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210220050731-9a76102bfb43/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -1440,6 +1450,7 @@ golang.org/x/tools v0.0.0-20200904185747-39188db58858/go.mod h1:Cj7w3i3Rnn0Xh82u golang.org/x/tools v0.0.0-20201110124207-079ba7bd75cd/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20201201161351-ac6f37ff4c2a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20201208233053-a543418bbed2/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= +golang.org/x/tools v0.0.0-20201224043029-2b0845dc783e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210105154028-b0ab187a4818/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.0.0-20210106214847-113979e3529a/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA= golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=