Enable wait for RepositoryServer CR to get ready functionality for actionsets #2228

r4rajat · 2023-07-26T09:38:35Z

Change Overview

Enable wait for resources to get ready functionality for actionsets

Add --wait-for-repository-server flag while Actionset creation, if provided waits for RepositoryServer to get in Ready state before creating Actionsets

Pull request type

Please check the type of change your PR introduces:

Issues

fixes #issue-number

Test Plan

💪 Manual
⚡ Unit test
💚 E2E

Manual Testing Steps

1) Build `kando` command line tool

$ go build -o kanctl ./cmd/kanctl/main.go

2) Create Actionset

./kanctl create actionset --action backup --namespace kanister --blueprint mysql-blueprint --statefulset mysql-test/mysql-release --secrets mysql=mysql-test/mysql-release --repository-server=kanister/kopia-repo-server-2 --wait-for-repository-server

<waits for repository server to get ready>
actionset backup-9b74s created

Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>

github-actions · 2023-07-26T09:38:49Z

Thanks for submitting this pull request 🎉. The team will review it soon and get back to you.

If you haven't already, please take a moment to review our project contributing guideline and Code of Conduct document.

PrasadG193 · 2023-07-26T10:11:12Z

pkg/kanctl/actionset.go

@@ -105,6 +107,7 @@ func newActionSetCmd() *cobra.Command {
 	cmd.Flags().String(selectorNamespaceFlag, "", "namespace to apply selector on. Used along with the selector specified using --selector/-l")
 	cmd.Flags().StringSliceP(namespaceTargetsFlagName, "T", []string{}, "namespaces for the action set, comma separated list of namespaces (eg: --namespacetargets namespace1,namespace2)")
 	cmd.Flags().StringSliceP(objectsFlagName, "O", []string{}, "objects for the action set, comma separated list of object references (eg: --objects group/version/resource/namespace1/name1,group/version/resource/namespace2/name2)")
+	cmd.Flags().BoolP(waitForReadyFlagName, "w", false, "wait for resources to get in ready state")


The wait flag seems a bit ambiguous. Is it for waiting for actionset to complete, or repo server to be ready or resources to be ready? We can have something --wait-for-reposerver?

PrasadG193 · 2023-07-26T10:12:38Z

pkg/kanctl/actionset.go

@@ -121,7 +124,7 @@ func initializeAndPerform(cmd *cobra.Command, args []string) error {
 	ctx := context.Background()
 	valFlag, _ := cmd.Flags().GetBool(skipValidationFlag)
 	if !valFlag {
-		err = verifyParams(ctx, params, cli, crCli, osCli)
+		err = verifyParams(cmd, ctx, params, cli, crCli, osCli)


instead of passing whole cmd object just to get value to wait flag can we parse it here and just pass the boolean parameter?

PrasadG193 · 2023-07-26T10:13:50Z

pkg/kanctl/actionset.go

+		if wait {
+			err = waitForKopiaRepositoryServerReady(ctx, crCli, rs)
+			if err != nil {
+				return err
+			}
+		} else {
+			if rs.Status.Progress != crv1alpha1.Ready {
+				err = errors.New("Repository Server Not Ready")
+				return errors.Wrapf(err, "Please make sure that Repository Server CR '%s' is in Ready State", repoServer.Name)
+			}
 		}


Suggested change

if wait {

err = waitForKopiaRepositoryServerReady(ctx, crCli, rs)

if err != nil {

return err

}

} else {

if rs.Status.Progress != crv1alpha1.Ready {

err = errors.New("Repository Server Not Ready")

return errors.Wrapf(err, "Please make sure that Repository Server CR '%s' is in Ready State", repoServer.Name)

}

}

if wait {

return waitForKopiaRepositoryServerReady(ctx, crCli, rs)

}

if rs.Status.Progress != crv1alpha1.Ready {

err = errors.New("Repository Server Not Ready")

return errors.Wrapf(err, "Please make sure that Repository Server CR '%s' is in Ready State", repoServer.Name)

}

We can get rid of else block

PrasadG193 · 2023-07-26T10:14:39Z

pkg/kanctl/actionset.go

+		if repositoryServer.Status.Progress == crv1alpha1.Ready && err == nil {
+			return true, nil
+		}
+		return false, err


Suggested change

if repositoryServer.Status.Progress == crv1alpha1.Ready && err == nil {

return true, nil

}

return false, err

return repositoryServer.Status.Progress == crv1alpha1.Ready, err

Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>

pavannd1 · 2023-07-26T22:11:07Z

pkg/kanctl/actionset.go

+	timeoutCtx, waitCancel := context.WithTimeout(ctx, contextWaitTimeout)
+	defer waitCancel()
+	pollErr := poll.Wait(timeoutCtx, func(ctx context.Context) (bool, error) {
+		repositoryServer, err := crCli.CrV1alpha1().RepositoryServers(rs.GetNamespace()).Get(ctx, rs.GetName(), metav1.GetOptions{})


We should continue checking if the supplied repo server doesn't exist right? Something like

if apierrors.IsNotFound(errors.Cause(err)) { return false, nil }

@pavannd1 we're calling this function i.e waitForKopiaRepositoryServerReady in verifyRepositoryServerParams function and there we're checking if the Repository Server is present or not.

So before polling, we're making sure that RepositoryServer CR exists

pkg/kanctl/actionset.go

pavannd1 · 2023-07-26T22:13:16Z

pkg/kanctl/actionset.go

@@ -755,7 +759,7 @@ func generateActionSetName(p *PerformParams) (string, error) {
 	return "", errMissingFieldActionName
 }

-func verifyRepositoryServerParams(repoServer *crv1alpha1.ObjectReference, crCli versioned.Interface, ctx context.Context) error {
+func verifyRepositoryServerParams(waitForRepoServerReady bool, repoServer *crv1alpha1.ObjectReference, crCli versioned.Interface, ctx context.Context) error {


Not in this PR but we should change the order of arguments here. The convention is

context, clis..., other args...

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>

Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>

PrasadG193

LGTM

Enable wait for resources to get ready functionality for actionsets

cff6a5f

Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>

infraq added this to In Progress in Kanister Jul 26, 2023

r4rajat requested review from pavannd1, PrasadG193 and kale-amruta July 26, 2023 09:45

PrasadG193 reviewed Jul 26, 2023

View reviewed changes

r4rajat added 2 commits July 26, 2023 19:14

rename waitForRepositoryServerReadyFlagName value

9a2349a

Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>

Remove unnecessary else in code block

661820b

Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>

r4rajat requested a review from PrasadG193 July 26, 2023 13:49

pavannd1 reviewed Jul 26, 2023

View reviewed changes

r4rajat and others added 2 commits July 27, 2023 12:33

Update pkg/kanctl/actionset.go

49b7b1b

Co-authored-by: Pavan Navarathna <6504783+pavannd1@users.noreply.github.com>

Change order of arguments in function verifyRepositoryServerParams

3c841cd

Signed-off-by: Rajat Gupta <rajat.gupta@veeam.com>

r4rajat requested a review from pavannd1 July 27, 2023 07:11

r4rajat changed the title ~~Enable wait for resources to get ready functionality for actionsets~~ Enable wait for RepositoryServer CR to get ready functionality for actionsets Jul 28, 2023

PrasadG193 approved these changes Jul 28, 2023

View reviewed changes

Kanister automation moved this from In Progress to Reviewer approved Jul 28, 2023

r4rajat added the kueue label Jul 28, 2023

Merge branch 'master' into enable_wait_in_actionset

92df2c9

r4rajat assigned PrasadG193 Jul 28, 2023

mergify bot merged commit 7df4275 into master Jul 28, 2023
14 checks passed

Kanister automation moved this from Reviewer approved to Done Jul 28, 2023

mergify bot deleted the enable_wait_in_actionset branch July 28, 2023 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable wait for RepositoryServer CR to get ready functionality for actionsets #2228

Enable wait for RepositoryServer CR to get ready functionality for actionsets #2228

r4rajat commented Jul 26, 2023 •

edited

Loading

github-actions bot commented Jul 26, 2023

PrasadG193 Jul 26, 2023

r4rajat Jul 26, 2023

PrasadG193 Jul 26, 2023

r4rajat Jul 26, 2023

PrasadG193 Jul 26, 2023

r4rajat Jul 26, 2023

PrasadG193 Jul 26, 2023

r4rajat Jul 26, 2023

pavannd1 Jul 26, 2023

r4rajat Jul 27, 2023

pavannd1 Jul 26, 2023

r4rajat Jul 27, 2023

PrasadG193 left a comment

Enable wait for RepositoryServer CR to get ready functionality for actionsets #2228

Enable wait for RepositoryServer CR to get ready functionality for actionsets #2228

Conversation

r4rajat commented Jul 26, 2023 • edited Loading

Change Overview

Pull request type

Issues

Test Plan

Manual Testing Steps

1) Build kando command line tool

2) Create Actionset

github-actions bot commented Jul 26, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PrasadG193 left a comment

Choose a reason for hiding this comment

r4rajat commented Jul 26, 2023 •

edited

Loading

1) Build `kando` command line tool