Make bastion hosts reconcile/delete async by Jont828 · Pull Request #1941 · kubernetes-sigs/cluster-api-provider-azure

Jont828 · 2021-12-20T23:31:29Z

What type of PR is this?

/kind feature
What this PR does / why we need it: Implementation of an async service for bastion hosts as part of an effort to make all services async. See #1610 and #1541.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #1710

Special notes for your reviewer:

Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

squashed commits
includes documentation
adds unit tests

Release note:

Make bastion hosts reconcile/delete async

k8s-ci-robot · 2021-12-20T23:31:37Z

Hi @Jont828. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

CecileRobertMichon · 2022-01-08T01:09:49Z

/ok-to-test

devigned

lgtm!

CecileRobertMichon

@Jont828 are we testing the AzureBastion feature in any of the e2e test templates? i.e. are the modified code paths already covered by a test or should we run a manual test to make sure there are no issues

azure/scope/cluster.go

CecileRobertMichon · 2022-01-19T01:17:08Z

@Jont828 make sure you remove WIP from the title when this is ready

CecileRobertMichon · 2022-01-19T23:31:20Z

azure/services/bastionhosts/bastionhosts.go

 // Service provides operations on Azure resources.
 type Service struct {
 	Scope BastionScope
 	client


do we still need client in this struct?

azure/services/bastionhosts/bastionhosts.go

CecileRobertMichon · 2022-01-19T23:36:40Z

azure/services/bastionhosts/bastionhosts.go

 	}
-	return nil
+
+	s.Scope.UpdateDeleteStatus(infrav1.BastionHostReadyCondition, serviceName, resultingErr)


right now this will update the status even if there is no bastion associated with the cluster, needs to be addressed as part of #1868

azure/services/bastionhosts/bastionhosts.go

templates/test/ci/prow-private/patches/bastion.yaml

Jont828 · 2022-01-21T22:47:34Z

/retest

Jont828 · 2022-01-24T17:40:02Z

/retest

azure/services/bastionhosts/client.go

templates/test/ci/cluster-template-prow-private.yaml

k8s-ci-robot · 2022-01-24T23:25:11Z

@Jont828: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-cluster-api-provider-azure-apidiff	`7916725`	link	false	`/test pull-cluster-api-provider-azure-apidiff`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

CecileRobertMichon

/lgtm
/approve

🚀

k8s-ci-robot · 2022-01-25T00:04:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: CecileRobertMichon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [CecileRobertMichon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 20, 2021

Jont828 changed the title ~~Make bastion hosts reconcile/delete async~~ Make bastion hosts reconcile/delete async [WIP] Dec 20, 2021

k8s-ci-robot requested review from CecileRobertMichon and shysank December 20, 2021 23:31

k8s-ci-robot added area/provider/azure Issues or PRs related to azure provider sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. labels Dec 20, 2021

Jont828 mentioned this pull request Dec 22, 2021

REQUEST: New membership for Jont828 kubernetes/org#3156

Closed

7 tasks

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 22, 2021

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jan 8, 2022

Jont828 force-pushed the async-bastionhosts branch from 3720dc2 to ee23959 Compare January 12, 2022 00:16

devigned approved these changes Jan 12, 2022

View reviewed changes

CecileRobertMichon reviewed Jan 12, 2022

View reviewed changes

Jont828 force-pushed the async-bastionhosts branch from ee23959 to 72a2841 Compare January 12, 2022 23:04

CecileRobertMichon reviewed Jan 13, 2022

View reviewed changes

azure/scope/cluster.go Outdated Show resolved Hide resolved

Jont828 force-pushed the async-bastionhosts branch 2 times, most recently from b35c24c to 0034635 Compare January 14, 2022 20:43

Jont828 changed the title ~~Make bastion hosts reconcile/delete async [WIP]~~ Make bastion hosts reconcile/delete async Jan 19, 2022

Jont828 force-pushed the async-bastionhosts branch from 47e7d52 to 81796d1 Compare January 19, 2022 22:18

CecileRobertMichon reviewed Jan 19, 2022

View reviewed changes

azure/services/bastionhosts/bastionhosts.go Outdated Show resolved Hide resolved

CecileRobertMichon reviewed Jan 19, 2022

View reviewed changes

CecileRobertMichon reviewed Jan 20, 2022

View reviewed changes

azure/services/bastionhosts/bastionhosts.go Show resolved Hide resolved

Jont828 force-pushed the async-bastionhosts branch 3 times, most recently from c3f4b84 to c2cd3ca Compare January 21, 2022 00:39

CecileRobertMichon reviewed Jan 21, 2022

View reviewed changes

templates/test/ci/prow-private/patches/bastion.yaml Outdated Show resolved Hide resolved

Jont828 force-pushed the async-bastionhosts branch from c2cd3ca to 3b583fc Compare January 21, 2022 20:03

Jont828 force-pushed the async-bastionhosts branch from 3b583fc to 5046e50 Compare January 24, 2022 18:09

CecileRobertMichon reviewed Jan 24, 2022

View reviewed changes

azure/services/bastionhosts/client.go Outdated Show resolved Hide resolved

CecileRobertMichon reviewed Jan 24, 2022

View reviewed changes

templates/test/ci/cluster-template-prow-private.yaml Show resolved Hide resolved

Jont828 force-pushed the async-bastionhosts branch 3 times, most recently from deb97db to 6f7e687 Compare January 24, 2022 23:18

Make bastion hosts reconcile/delete async

7916725

Jont828 force-pushed the async-bastionhosts branch from 6f7e687 to 7916725 Compare January 24, 2022 23:23

CecileRobertMichon reviewed Jan 25, 2022

View reviewed changes

k8s-ci-robot assigned CecileRobertMichon Jan 25, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 25, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 25, 2022

k8s-ci-robot merged commit 171b34c into kubernetes-sigs:main Jan 25, 2022

k8s-ci-robot added this to the v1.1 milestone Jan 25, 2022

Comments

Conversation

Jont828 commented Dec 20, 2021

Uh oh!

k8s-ci-robot commented Dec 20, 2021

Uh oh!

CecileRobertMichon commented Jan 8, 2022

Uh oh!

devigned left a comment

Choose a reason for hiding this comment

Uh oh!

CecileRobertMichon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CecileRobertMichon commented Jan 19, 2022

Uh oh!

CecileRobertMichon Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

CecileRobertMichon Jan 19, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Jont828 commented Jan 21, 2022

Uh oh!

Jont828 commented Jan 24, 2022

Uh oh!

Uh oh!

Uh oh!

k8s-ci-robot commented Jan 24, 2022

Uh oh!

CecileRobertMichon left a comment

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Jan 25, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants