Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add v1alpha2 API #673

Merged
merged 1 commit into from
Mar 20, 2023
Merged

Conversation

chrisdoherty4
Copy link
Member

@chrisdoherty4 chrisdoherty4 commented Feb 22, 2023

Design: https://github.com/tinkerbell/roadmap/blob/main/design/20230222_tinkerbell_crd_refactor.md

This PR adds the new v1alpha2 API. The APIs are configured as unserved meaning consumers won't be able to request the v1alpha2 version yet.

Webhooks will be added in a separate PR.

@chrisdoherty4 chrisdoherty4 force-pushed the feat/crd-refactor branch 5 times, most recently from 5cb6c2d to 7044386 Compare March 7, 2023 15:13
@chrisdoherty4 chrisdoherty4 force-pushed the feat/crd-refactor branch 2 times, most recently from 052a997 to 7c5726e Compare March 17, 2023 02:46
@chrisdoherty4 chrisdoherty4 marked this pull request as ready for review March 17, 2023 02:47
@chrisdoherty4 chrisdoherty4 force-pushed the feat/crd-refactor branch 4 times, most recently from 71f0674 to 25f2ad9 Compare March 17, 2023 13:04
@chrisdoherty4
Copy link
Member Author

Struggling to work out why List types keep getting generated.

@moadqassem
Copy link
Member

Struggling to work out why List types keep getting generated.

Do you mean this here:
https://github.com/tinkerbell/tink/pull/673/files#diff-929a63a497434edd4d1e013bf858fffbdba787e6b05c032321c1f634faac9414

@chrisdoherty4
Copy link
Member Author

chrisdoherty4 commented Mar 17, 2023

@moadqassem Yes. I've never seen a dedicated list CRD generated. Normally the list type is part of the CRD definition. I'm guessing I have something funky with my annotations.

I generated a new project with kubebuilder and did a quick cross reference but couldn't see anything.

@moadqassem
Copy link
Member

moadqassem commented Mar 17, 2023

@moadqassem Yes. I've never seen a dedicated list CRD generated. Normally the list type is part of the CRD definition. I'm guessing I have something funky with my annotations.

I generated a new project with kubebuilder and did a quick cross reference but couldn't see anything.

Did you try adding +kubebuilder:object:generate=false to your Objects ?

For example:

// +kubebuilder:object:generate=false
// +kubebuilder:object:root=true
// +kubebuilder:storageversion

// HardwareList contains a list of Hardware.
type HardwareList struct {
	metav1.TypeMeta `json:",inline"`
	metav1.ListMeta `json:"metadata,omitempty"`
	Items           []Hardware `json:"items"`
}

@chrisdoherty4
Copy link
Member Author

@moadqassem I'll give that a go, but that didn't exist in our v1alpha1 API and reflecting on other projects they don't seem to have it either (and they don't have list resources).

@moadqassem
Copy link
Member

@moadqassem I'll give that a go, but that didn't exist in our v1alpha1 API and reflecting on other projects they don't seem to have it either (and they don't have list resources).

I agree, would check also the genclient version, maybe something changed.

@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #673 (616a9c5) into main (b9619ec) will decrease coverage by 1.04%.
The diff coverage is n/a.

❗ Current head 616a9c5 differs from pull request most recent head c6f4499. Consider uploading reports for the commit c6f4499 to get more accurate results

@@            Coverage Diff             @@
##             main     #673      +/-   ##
==========================================
- Coverage   49.30%   48.26%   -1.04%     
==========================================
  Files          18       18              
  Lines         860      951      +91     
==========================================
+ Hits          424      459      +35     
- Misses        428      484      +56     
  Partials        8        8              

see 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@chrisdoherty4 chrisdoherty4 force-pushed the feat/crd-refactor branch 6 times, most recently from 412d297 to e821e13 Compare March 18, 2023 03:15
@chrisdoherty4
Copy link
Member Author

@moadqassem We have lift off 🚀 .

@chrisdoherty4 chrisdoherty4 force-pushed the feat/crd-refactor branch 2 times, most recently from 26128a2 to b366277 Compare March 18, 2023 04:10
Signed-off-by: Chris Doherty <[email protected]>
Copy link
Member

@jacobweinstock jacobweinstock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@chrisdoherty4 chrisdoherty4 added the ready-to-merge Signal to Mergify to merge the PR. label Mar 20, 2023
@mergify mergify bot merged commit 1606d94 into tinkerbell:main Mar 20, 2023
jacobweinstock added a commit to jacobweinstock/smee that referenced this pull request Mar 25, 2023
This functionality to disable DHCP entirely
will be available in the refactored hardware API.
See tinkerbell/tink#673

Signed-off-by: Jacob Weinstock <[email protected]>
@chrisdoherty4 chrisdoherty4 deleted the feat/crd-refactor branch May 15, 2023 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Signal to Mergify to merge the PR.
Projects
Development

Successfully merging this pull request may close these issues.

3 participants