✨ Markers can now indicate their priority when applying #706

dprotaso · 2022-07-15T17:30:22Z

There's a need for more control over when SchemaMarkers are applied to a Schema. Right now there are two levels - 'First' and everything else. The iteration over the markers within this level is non-deterministic because we're iterating over a golang map type.

As an alternative we now collect all the markers and sort them based on a new priority. Markers can signal their priority by implementing the optional interface

type ApplyPriority int64

type ApplyPriorityMarker interface {
 	ApplyPriority() ApplyPriority
 }

This change is backwards compatible - downstream markers that implement ApplyFirst() will behave the same as before.

k8s-ci-robot · 2022-07-15T17:30:31Z

Welcome @dprotaso!

It looks like this is your first PR to kubernetes-sigs/controller-tools 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/controller-tools has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2022-07-15T17:30:31Z

Hi @dprotaso. 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.

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.

dprotaso · 2022-07-15T17:38:03Z

/assign @vincepri @alvaroaleman

vincepri · 2022-08-24T02:01:26Z

/ok-to-test

dprotaso · 2022-10-17T14:51:35Z

Hey folks, just following up - is there anything else needed for this PR?

pkg/crd/markers/doc.go

dprotaso · 2022-12-05T18:11:39Z

@camilamacedo86 done - take a look

pkg/crd/markers/doc.go

camilamacedo86

I am ok with but would be nice get thoughts from others like @alvaroaleman @varshaprasad96

Otherwise, lgtm

dprotaso · 2023-01-17T20:50:46Z

Hi - folks just following up

camilamacedo86

It seems good or me. 🥇
Let's see what others think about

/lgtm

dprotaso · 2023-06-19T23:35:42Z

Prow started working again

@vincepri and/or @alvaroaleman PTAL

k8s-triage-robot · 2023-07-20T00:13:19Z

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

This bot triages 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 PR is closed

You can:

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

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

/lifecycle rotten

dprotaso · 2023-08-18T00:30:52Z

@vincepri and/or @alvaroaleman PTAL

dprotaso · 2023-10-23T14:08:23Z

@vincepri @alvaroaleman can you take a look please?

dprotaso · 2024-01-15T15:08:03Z

/lifecycle frozen

k8s-ci-robot · 2024-01-15T15:08:05Z

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

In response to this:

/lifecycle frozen

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.

dprotaso · 2024-01-15T15:08:48Z

/remove-lifecycle rotten

dprotaso · 2024-01-15T15:09:33Z

@vincepri @joelanford @alvaroaleman @sbueringer - is there anything holding up this PR? I've addressed the initial PR reviews.

sbueringer

Sorry for the long delay. Thx looks good. Just a few minor findings otherwise good to go from my side

pkg/crd/schema.go

pkg/crd/schema_test.go

pkg/crd/markers/priority.go

pkg/crd/markers/doc.go

sbueringer · 2024-01-31T18:05:19Z

Thank you!

/lgtm

/assign @alvaroaleman @vincepri

k8s-ci-robot · 2024-01-31T18:05:25Z

LGTM label has been added.

Git tree hash: 8f0d552eda3a900ac0aaf3c11be5cc66565bc7ce

sbueringer · 2024-02-05T10:29:21Z

/approve

Thank you very much!

k8s-ci-robot · 2024-02-05T10:29:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, sbueringer

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [sbueringer]

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

…igs#706) * Markers can now indicate their priority when applying * add unit test to ensure marker invocation * add example in godoc * drop unused struct param * update godoc * run goimports * change marker constants * add godoc comment * address feedback * add comment that ordering is for validation markers for now * fix godoc comment

Markers can now indicate their priority when applying

8df4154

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 15, 2022

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 15, 2022

k8s-ci-robot requested review from mengqiy and pwittrock July 15, 2022 17:30

dprotaso changed the title ~~Markers can now indicate their priority when applying~~ ✨Markers can now indicate their priority when applying Jul 15, 2022

dprotaso changed the title ~~✨Markers can now indicate their priority when applying~~ ✨ Markers can now indicate their priority when applying Jul 15, 2022

k8s-ci-robot assigned alvaroaleman and vincepri Jul 15, 2022

add unit test to ensure marker invocation

2a3e4d9

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 Aug 24, 2022

camilamacedo86 reviewed Nov 24, 2022

View reviewed changes

pkg/crd/markers/doc.go Outdated Show resolved Hide resolved

add example in godoc

4710a92

drop unused struct param

43c8282

camilamacedo86 reviewed Dec 6, 2022

View reviewed changes

pkg/crd/markers/doc.go Outdated Show resolved Hide resolved

camilamacedo86 reviewed Dec 6, 2022

View reviewed changes

dprotaso added 2 commits December 6, 2022 12:08

update godoc

bc48feb

run goimports

11c6121

camilamacedo86 reviewed Jan 18, 2023

View reviewed changes

k8s-ci-robot assigned camilamacedo86 Jan 18, 2023

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

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 Jul 20, 2023

k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Jan 15, 2024

sbueringer reviewed Jan 23, 2024

View reviewed changes

pkg/crd/schema.go Outdated Show resolved Hide resolved

pkg/crd/schema.go Outdated Show resolved Hide resolved

pkg/crd/schema_test.go Outdated Show resolved Hide resolved

pkg/crd/schema_test.go Outdated Show resolved Hide resolved

pkg/crd/markers/priority.go Show resolved Hide resolved

address feedback

852fc90

sbueringer reviewed Jan 26, 2024

View reviewed changes

pkg/crd/markers/doc.go Outdated Show resolved Hide resolved

dprotaso added 2 commits January 31, 2024 13:01

add comment that ordering is for validation markers for now

f9cffe3

fix godoc comment

239ff67

sbueringer added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Jan 31, 2024

k8s-ci-robot assigned sbueringer Jan 31, 2024

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

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 5, 2024

k8s-ci-robot merged commit b67d943 into kubernetes-sigs:master Feb 5, 2024
3 checks passed

renovate bot mentioned this pull request Jun 4, 2024

fix(deps): update godeps - autoclosed stakater/provider-keycloak#16

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Markers can now indicate their priority when applying #706

✨ Markers can now indicate their priority when applying #706

dprotaso commented Jul 15, 2022 •

edited

Loading

k8s-ci-robot commented Jul 15, 2022

k8s-ci-robot commented Jul 15, 2022

dprotaso commented Jul 15, 2022

vincepri commented Aug 24, 2022

dprotaso commented Oct 17, 2022

dprotaso commented Dec 5, 2022

camilamacedo86 left a comment

dprotaso commented Jan 17, 2023

camilamacedo86 left a comment

dprotaso commented Jun 19, 2023

k8s-triage-robot commented Jul 20, 2023

dprotaso commented Aug 18, 2023

dprotaso commented Oct 23, 2023

dprotaso commented Jan 15, 2024

k8s-ci-robot commented Jan 15, 2024

dprotaso commented Jan 15, 2024

dprotaso commented Jan 15, 2024

sbueringer left a comment •

edited

Loading

sbueringer commented Jan 31, 2024

k8s-ci-robot commented Jan 31, 2024

sbueringer commented Feb 5, 2024

k8s-ci-robot commented Feb 5, 2024

✨ Markers can now indicate their priority when applying #706

✨ Markers can now indicate their priority when applying #706

Conversation

dprotaso commented Jul 15, 2022 • edited Loading

k8s-ci-robot commented Jul 15, 2022

k8s-ci-robot commented Jul 15, 2022

dprotaso commented Jul 15, 2022

vincepri commented Aug 24, 2022

dprotaso commented Oct 17, 2022

dprotaso commented Dec 5, 2022

camilamacedo86 left a comment

Choose a reason for hiding this comment

dprotaso commented Jan 17, 2023

camilamacedo86 left a comment

Choose a reason for hiding this comment

dprotaso commented Jun 19, 2023

k8s-triage-robot commented Jul 20, 2023

dprotaso commented Aug 18, 2023

dprotaso commented Oct 23, 2023

dprotaso commented Jan 15, 2024

k8s-ci-robot commented Jan 15, 2024

dprotaso commented Jan 15, 2024

dprotaso commented Jan 15, 2024

sbueringer left a comment • edited Loading

Choose a reason for hiding this comment

sbueringer commented Jan 31, 2024

k8s-ci-robot commented Jan 31, 2024

sbueringer commented Feb 5, 2024

k8s-ci-robot commented Feb 5, 2024

dprotaso commented Jul 15, 2022 •

edited

Loading

sbueringer left a comment •

edited

Loading