✨ WIP: Add Kubernetes Gomega extension with to make testing controllers easier #1364

JoelSpeed · 2021-01-29T19:36:00Z

This is an initial attempt at solving #933. I've revamped the design a little from the original to try and take into account some changes to the client since I originally came up with this idea.

Essentially this is a Kubernetes wrapper on top of Gomega to make it easy to write tests using envtest.

I've marked this as WIP at the moment as I would like to get some feedback on the initial design (and naming), and I would like to add docs on how to use this as well as tests to work as proof that the matchers work and as examples on how to implement it.

Add docs
Add tests

The outcome here is that inside a test, I should be able to write something like:

m.WithTimeout(timeout).Eventually(workerNode1).Should(WithField("Spec.Taints",
	ContainElement(SatisfyAll(
		utils.WithField("Effect", Equal(corev1.TaintEffect("NoSchedule"))),
		utils.WithField("Key", Equal("node.kubernetes.io/unschedulable")),
	)),
))

To check that my controller eventually reconciles my node and adds the taint specified. Or yanno, whatever you want to check. Internally, this will poll until the Get returns no errors and then return the node, and with the WithField transform, allow me to extract particular fields and assert their values.

This adds a utility that is intended to be used with envtest to make it easier for users to write tests. The Matcher wraps common operations that you might do with gomega when interacting with Kubernetes to allow simpler test assertions.

This should allow people to access fields within their objects fetched by the Matcher. This allows easier assertions by allowing specific fields to be compared during tests.

k8s-ci-robot · 2021-01-29T19:36:01Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

k8s-ci-robot · 2021-01-29T19:36:10Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: JoelSpeed
To complete the pull request process, please assign vincepri after the PR has been reviewed.
You can assign the PR to them by writing /assign @vincepri in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

JoelSpeed · 2021-01-29T19:37:14Z

/assign @vincepri Since I know you commented on the original issue I proposed. Looking for initial feedback on if this is something we still want to do and whether the interfaces seem reasonable.

Danil-Grigorev · 2021-02-12T17:16:34Z

pkg/envtest/komega/matcher.go

+)
+
+// Matcher has Gomega Matchers that use the controller-runtime client.
+type Matcher struct {


I think Matcher should be unexported here, and the initialized struct should be returned as a non-pointer value from a builder method. With a repetitive and parallel nature of tests, it will be good to always know you work with unique komega interface using exact client, extras or timeout.

Hmm that's a good point, at the moment we are using all pointer receivers, which means we will affect the parent. Perhaps we need to stop doing that and then whenever you call one of the builder style methods, it would not affect the parent it's being called upon, means you could compose different settings for each call if need be

I expect people will create a new matcher for each test anyway (either in a BeforeEach in Ginkgo or at the start of the test in normal test suites), a NewMatcher method might make this cleaner though, good suggestion

JoelSpeed · 2021-02-15T10:03:14Z

pkg/envtest/komega/matcher.go

+)
+
+// Matcher has Gomega Matchers that use the controller-runtime client.
+type Matcher struct {


Hmm that's a good point, at the moment we are using all pointer receivers, which means we will affect the parent. Perhaps we need to stop doing that and then whenever you call one of the builder style methods, it would not affect the parent it's being called upon, means you could compose different settings for each call if need be

I expect people will create a new matcher for each test anyway (either in a BeforeEach in Ginkgo or at the start of the test in normal test suites), a NewMatcher method might make this cleaner though, good suggestion

JoelSpeed · 2021-02-15T10:06:13Z

pkg/envtest/komega/matcher.go

+// Create creates the object on the API server.
+func (m *Matcher) Create(obj client.Object, opts ...client.CreateOption) gomega.GomegaAssertion {
+	err := m.Client.Create(m.context(), obj, opts...)
+	return gomega.Expect(err, m.extras...)


Currently, these will only work with the default package level gomega assertions, should make the option of passing in a custom gomega.WithT as well to this can be used in normal go testing tests

fejta-bot · 2021-05-16T10:54:14Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-contributor-experience at kubernetes/community.
/lifecycle stale

JoelSpeed · 2021-05-17T09:22:22Z

/remove-lifecycle stale

Would still like to contribute this at some point if it's useful. Additional feedback on the initial design would be appreciated

k8s-triage-robot · 2021-08-15T09:25:58Z

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle stale
Mark this issue or PR as rotten with /lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

k8s-triage-robot · 2021-09-14T09:37:43Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Mark this issue or PR as fresh with /remove-lifecycle rotten
Close this issue or PR with /close
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

joelanford · 2021-09-14T16:27:01Z

/lifecycle frozen

This seems pretty useful, so I don't want this to be auto-closed by the ci-robot.

@JoelSpeed is this still a WIP? If you're not actively making changes, it might be good to remove the WIP designation to signal reviewers that its ready for review.

k8s-ci-robot · 2021-09-14T16:27:02Z

@joelanford: The lifecycle/frozen label cannot be applied to Pull Requests.

In response to this:

/lifecycle frozen

This seems pretty useful, so I don't want this to be auto-closed by the ci-robot.

@JoelSpeed is this still a WIP? If you're not actively making changes, it might be good to remove the WIP designation to signal reviewers that its ready for review.

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.

JoelSpeed · 2021-09-15T08:16:09Z

There's some feedback that I wanted to address about what's exported that probably shouldn't be, I'll try and get to that this week, then I'll remove the WIP tag

k8s-triage-robot · 2021-10-15T08:21:54Z

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied
After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen
Mark this issue or PR as fresh with /remove-lifecycle rotten
Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

k8s-ci-robot · 2021-10-15T08:22:05Z

@k8s-triage-robot: Closed this PR.

In response to this:

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

After 90d of inactivity, lifecycle/stale is applied

After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied

After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

Reopen this issue or PR with /reopen

Mark this issue or PR as fresh with /remove-lifecycle rotten

Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/close

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.

JoelSpeed added 2 commits January 29, 2021 19:22

Add Komega matcher and interfaces

4f20d07

This adds a utility that is intended to be used with envtest to make it easier for users to write tests. The Matcher wraps common operations that you might do with gomega when interacting with Kubernetes to allow simpler test assertions.

Add WithField transformer

38101e5

This should allow people to access fields within their objects fetched by the Matcher. This allows easier assertions by allowing specific fields to be compared during tests.

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 29, 2021

k8s-ci-robot requested review from droot and joelanford January 29, 2021 19:36

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 29, 2021

Danil-Grigorev reviewed Feb 12, 2021

View reviewed changes

JoelSpeed commented Feb 15, 2021

View reviewed changes

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 16, 2021

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 17, 2021

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 15, 2021

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 14, 2021

killianmuldoon mentioned this pull request Sep 27, 2021

Integrate internal/matchers into Komega style test abstraction in Controller Runtime kubernetes-sigs/cluster-api#5338

Closed

fabriziopandini mentioned this pull request Sep 30, 2021

Create unified patching/matching library kubernetes-sigs/cluster-api#5316

Closed

k8s-ci-robot closed this Oct 15, 2021

schrej mentioned this pull request Jan 6, 2022

✨ Add Kubernetes Gomega extension with to make testing controllers easier #1767

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ WIP: Add Kubernetes Gomega extension with to make testing controllers easier #1364

✨ WIP: Add Kubernetes Gomega extension with to make testing controllers easier #1364

JoelSpeed commented Jan 29, 2021

k8s-ci-robot commented Jan 29, 2021

k8s-ci-robot commented Jan 29, 2021

JoelSpeed commented Jan 29, 2021

Danil-Grigorev Feb 12, 2021

JoelSpeed Feb 15, 2021

JoelSpeed Feb 15, 2021

JoelSpeed Feb 15, 2021

fejta-bot commented May 16, 2021

JoelSpeed commented May 17, 2021

k8s-triage-robot commented Aug 15, 2021

k8s-triage-robot commented Sep 14, 2021

joelanford commented Sep 14, 2021

k8s-ci-robot commented Sep 14, 2021

JoelSpeed commented Sep 15, 2021

k8s-triage-robot commented Oct 15, 2021

k8s-ci-robot commented Oct 15, 2021

✨ WIP: Add Kubernetes Gomega extension with to make testing controllers easier #1364

✨ WIP: Add Kubernetes Gomega extension with to make testing controllers easier #1364

Conversation

JoelSpeed commented Jan 29, 2021

k8s-ci-robot commented Jan 29, 2021

k8s-ci-robot commented Jan 29, 2021

JoelSpeed commented Jan 29, 2021

Danil-Grigorev Feb 12, 2021

Choose a reason for hiding this comment

JoelSpeed Feb 15, 2021

Choose a reason for hiding this comment

JoelSpeed Feb 15, 2021

Choose a reason for hiding this comment

JoelSpeed Feb 15, 2021

Choose a reason for hiding this comment

fejta-bot commented May 16, 2021

JoelSpeed commented May 17, 2021

k8s-triage-robot commented Aug 15, 2021

k8s-triage-robot commented Sep 14, 2021

joelanford commented Sep 14, 2021

k8s-ci-robot commented Sep 14, 2021

JoelSpeed commented Sep 15, 2021

k8s-triage-robot commented Oct 15, 2021

k8s-ci-robot commented Oct 15, 2021