✨ Expose CRDInstallOptions via envtest.Environment #544
Conversation
f18db79 to
f339933
Compare
|
/assign @DirectXMan12 |
pkg/envtest/server.go
Outdated
| // If both this field and Paths field in CRDInstallOptions are specified, this | ||
| // value this ignored. | ||
| // | ||
| // Deprecated: Use CRDInstallOptions |
There was a problem hiding this comment.
let's not deprecate these -- they're way more convenient when constructing the object directly -- constructing embedded objects is a pain.
There was a problem hiding this comment.
Sounds reasonable.
pkg/envtest/server.go
Outdated
| te.CRDInstallOptions.CRDs = te.CRDs | ||
| } | ||
| if len(te.CRDInstallOptions.Paths) == 0 { | ||
| te.CRDInstallOptions.Paths = te.CRDDirectoryPaths |
There was a problem hiding this comment.
can't we just append here?
There was a problem hiding this comment.
Sure, will do. Do we need to filter out "duplicate" paths and CRDs though? In case the same values are passed both directly and via CRDInstallOptions?
There was a problem hiding this comment.
Just to clarify, "duplicate" CRDs mean they contain the same name right? I don't have to DeepEqual or anything on those CRD objects?
There was a problem hiding this comment.
identical name is sufficient. Duplicate name with different contents is an error, so we don't need to try and support that.
f339933 to
fd5c30a
Compare
This change provides better control for installing CRDs in `envtest`. Addresses kubernetes-sigs#541
fd5c30a to
c6bbb02
Compare
|
/ok-to-test |
|
let's see if we can re-trigger the tests /retest |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DirectXMan12, rajathagasthya 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:
Approvers can indicate their approval by writing |
Update book README, fix a typo and formatting
This change provides better control for installing CRDs in
envtest.Fixes #541