search

openshift / aws-account-operator

Operator to manage pool of AWS accounts for Hive
Apache License 2.0
32 stars 75 forks source link

Get rid of `operator-sdk generate` #585

Closed 2uasimojo closed 3 years ago

2uasimojo commented 3 years ago

PR #580 replaced operator-sdk generate crds with a controller-gen command (plus some hacks to make it match up with what operator-sdk did). This commit similarly replaces operator-sdk generate k8s with a controller-gen command, reducing our dependency on operator-sdk to just go imports.

While I was in here, I noticed that some framework-generated comments were making their way into the Description of some of the CRD fields and corresponding generated openapi. Nixed those.

Also cleaned up the redundant parts of generate_crds.sh which were running go and openapi generators. These were already part of the boilerplate-provided generate target as go-generate and openapi-generate.

Note that regenerating deepcopy code under controller-gen results in a trivial delta to each method whereby the implicit no-op return statement is removed.

TODO: Some of the "make compatible with operator-sdk" parts of generate_crds.sh are definitely not necessary (e.g. nothing cares whether the CRD files are named with _crd) and some are probably not necessary (I doubt anything breaks if things like annotations, creationTimestamp, status, and list type fields are included). If we can get rid of these, it should be easy enough to collapse this script back into one or two lines in the Makefile.

openshift-ci[bot] commented 3 years ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 2uasimojo To complete the pull request process, please assign rogbas after the PR has been reviewed. You can assign the PR to them by writing /assign @rogbas 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](https://github.com/openshift/aws-account-operator/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
codecov-commenter commented 3 years ago

Codecov Report

Merging #585 (f852163) into master (a572ba2) will decrease coverage by 0.21%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #585      +/-   ##
==========================================
- Coverage   13.59%   13.37%   -0.22%     
==========================================
  Files          45       45              
  Lines        3789     3850      +61     
==========================================
  Hits          515      515              
- Misses       3251     3312      +61     
  Partials       23       23
Impacted Files Coverage Δ
pkg/apis/aws/v1alpha1/account_types.go 90.47% <ø> (ø)
pkg/apis/aws/v1alpha1/accountclaim_types.go 8.33% <ø> (ø)
pkg/apis/aws/v1alpha1/accountpool_types.go 100.00% <ø> (ø)
pkg/controller/account/account_controller.go 0.36% <0.00%> (-0.03%) :arrow_down:
pkg/awsclient/client.go 0.00% <0.00%> (ø)
pkg/controller/account/ec2.go 0.00% <0.00%> (ø)
2uasimojo commented 3 years ago

Superseded by #588