Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Makes apis/run/v1alpha3 pkg a Go module #2222

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

akutz
Copy link
Contributor

@akutz akutz commented Apr 22, 2022

What this PR does / why we need it

This patch defines a new Go module for package apis/run/v1alpha3 so other projects may depend on this package without pulling in this project's entire dependency graph.

The reason the module is not defined for apis/run is because versions v1alpha1 and v1alpha2 depend back on this project, pulling in its dependency graph anyway.

Which issue(s) this PR fixes

Fixes NA

Describe testing done for PR

$ find . -name go.mod -print0 | \                                 
  xargs -0 dirname | \
  xargs -I% -n1 bash -c 'cd %; echo; echo "building $(pwd)"; time go build ./...; cd - 2>&1 >/dev/null'

building /Users/akutz/Projects/tanzu-framework

real	0m42.722s
user	5m30.315s
sys	1m21.629s

building /Users/akutz/Projects/tanzu-framework/apis/cpi

real	0m0.617s
user	0m0.572s
sys	0m1.017s

building /Users/akutz/Projects/tanzu-framework/apis/cni

real	0m1.267s
user	0m1.419s
sys	0m2.311s

building /Users/akutz/Projects/tanzu-framework/apis/run/v1alpha3

real	0m5.659s
user	0m29.953s
sys	0m8.068s

building /Users/akutz/Projects/tanzu-framework/addons

real	0m4.610s
user	0m7.124s
sys	0m3.993s

building /Users/akutz/Projects/tanzu-framework/addons/pinniped/post-deploy
go: downloading k8s.io/client-go v0.23.6

real	0m15.863s
user	1m17.030s
sys	0m15.790s

building /Users/akutz/Projects/tanzu-framework/addons/pinniped/config-controller

real	0m3.567s
user	0m3.888s
sys	0m3.147s

building /Users/akutz/Projects/tanzu-framework/hack/tools
go: warning: "./..." matched no packages

real	0m0.015s
user	0m0.005s
sys	0m0.005s

building /Users/akutz/Projects/tanzu-framework/hack/packages/kbld-image-replace

real	0m1.304s
user	0m1.253s
sys	0m1.234s

building /Users/akutz/Projects/tanzu-framework/hack/packages/package-tools

real	0m0.709s
user	0m0.790s
sys	0m0.723s

building /Users/akutz/Projects/tanzu-framework/pkg/v1/providers/tests

real	0m0.480s
user	0m0.501s
sys	0m0.868s

Release note

package-based-lcm: Changed the apis/run/v1alpha3 packages to be its own Go module so other projects can import its APIs without pulling in this project's entire dependency graph.

PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Related to #2216

cc @vijaykatam @sidharthsurana @jmoroski

Special notes for your reviewer

This patch defines a new Go module for package apis/run/v1alpha3
so other projects may depend on this package without pulling in this
project's entire dependency graph.

The reason the module is not defined for apis/run is because versions
v1alpha1 and v1alpha2 depend back on this project, pulling in its
dependency graph anyway.
@akutz
Copy link
Contributor Author

akutz commented Apr 22, 2022

This change may not in fact be possible due to kubernetes-sigs/controller-tools#516. The tl;dr is that you cannot currently split multiple API versions into distinct Go modules, or one of them will not be processed in the CRD generation. For example, this is why this PR's tests are failing -- because the v1alpha3 version of the TKR CRD is no longer generated and therefore no longer installed in cluster for testing:

diff --git a/config/crd/bases/run.tanzu.vmware.com_tanzukubernetesreleases.yaml b/config/crd/bases/run.tanzu.vmware.com_tanzukubernetesreleases.yaml
index cb9ef038..104b5e61 100644
--- a/config/crd/bases/run.tanzu.vmware.com_tanzukubernetesreleases.yaml
+++ b/config/crd/bases/run.tanzu.vmware.com_tanzukubernetesreleases.yaml
@@ -357,196 +357,6 @@ spec:
     storage: false
     subresources:
       status: {}
-  - additionalPrinterColumns:
-    - jsonPath: .spec.version
-      name: Version
-      type: string
-    - jsonPath: .status.conditions[?(@.type=='Compatible')].status
-      name: Compatible
-      type: string
-    - jsonPath: .metadata.creationTimestamp
-      name: Created
-      type: date
-    name: v1alpha3
-    schema:
-      openAPIV3Schema:
-        description: TanzuKubernetesRelease is the schema for the tanzukubernetesreleases
-          API. TanzuKubernetesRelease objects represent Kubernetes releases available
-          via TKG, which can be used to create TanzuKubernetesCluster instances. TKRs
-          are immutable to end-users. They are created and managed by TKG to provide
-          discovery of Kubernetes releases to TKG users.
-        properties:
-          apiVersion:
-            description: 'APIVersion defines the versioned schema of this representation
-              of an object. Servers should convert recognized schemas to the latest
-              internal value, and may reject unrecognized values. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources'
-            type: string
-          kind:
-            description: 'Kind is a string value representing the REST resource this
-              object represents. Servers may infer this from the endpoint the client
-              submits requests to. Cannot be updated. In CamelCase. More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds'
-            type: string
-          metadata:
-            type: object
-          spec:
-            description: TanzuKubernetesReleaseSpec defines the desired state of TanzuKubernetesRelease
-            properties:
-              bootstrapPackages:
-                description: BootstrapPackages lists references to all bootstrap packages
-                  shipped with this TKR.
-                items:
-                  description: LocalObjectReference contains enough information to
-                    let you locate the referenced object inside the same namespace.
-                  properties:
-                    name:
-                      description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
-                        TODO: Add other useful fields. apiVersion, kind, uid?'
-                      type: string
-                  type: object
-                type: array
-              kubernetes:
-                description: Kubernetes is Kubernetes
-                properties:
-                  coredns:
-                    description: CoreDNS specifies the container image repository
-                      and tag for coredns.
-                    properties:
-                      imageRepository:
-                        description: ImageRepository sets the container registry to
-                          pull images from. if not set, defaults to the ImageRepository
-                          defined in KubernetesSpec.
-                        type: string
-                      imageTag:
-                        description: ImageTag specifies a tag for the image.
-                        type: string
-                    type: object
-                  etcd:
-                    description: Etcd specifies the container image repository and
-                      tag for etcd.
-                    properties:
-                      imageRepository:
-                        description: ImageRepository sets the container registry to
-                          pull images from. if not set, defaults to the ImageRepository
-                          defined in KubernetesSpec.
-                        type: string
-                      imageTag:
-                        description: ImageTag specifies a tag for the image.
-                        type: string
-                    type: object
-                  imageRepository:
-                    description: ImageRepository specifies container image registry
-                      to pull images from.
-                    type: string
-                  kube-vip:
-                    description: KubeVIP specifies the container image repository
-                      and tag for kube-vip.
-                    properties:
-                      imageRepository:
-                        description: ImageRepository sets the container registry to
-                          pull images from. if not set, defaults to the ImageRepository
-                          defined in KubernetesSpec.
-                        type: string
-                      imageTag:
-                        description: ImageTag specifies a tag for the image.
-                        type: string
-                    type: object
-                  pause:
-                    description: Pause specifies the container image repository and
-                      tag for pause.
-                    properties:
-                      imageRepository:
-                        description: ImageRepository sets the container registry to
-                          pull images from. if not set, defaults to the ImageRepository
-                          defined in KubernetesSpec.
-                        type: string
-                      imageTag:
-                        description: ImageTag specifies a tag for the image.
-                        type: string
-                    type: object
-                  version:
-                    description: Version is Semantic Versioning conformant version
-                      of the Kubernetes build shipped by this TKR. The same Kubernetes
-                      build MAY be shipped by multiple TKRs.
-                    type: string
-                required:
-                - version
-                type: object
-              osImages:
-                description: OSImages lists references to all OSImage objects shipped
-                  with this TKR.
-                items:
-                  description: LocalObjectReference contains enough information to
-                    let you locate the referenced object inside the same namespace.
-                  properties:
-                    name:
-                      description: 'Name of the referent. More info: https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#names
-                        TODO: Add other useful fields. apiVersion, kind, uid?'
-                      type: string
-                  type: object
-                type: array
-              version:
-                description: Version is the fully qualified Semantic Versioning conformant
-                  version of the TanzuKubernetesRelease. Version MUST be unique across
-                  all TanzuKubernetesRelease objects.
-                type: string
-            required:
-            - kubernetes
-            - version
-            type: object
-          status:
-            description: TanzuKubernetesReleaseStatus defines the observed state of
-              TanzuKubernetesRelease
-            properties:
-              conditions:
-                items:
-                  description: Condition defines an observation of a Cluster API resource
-                    operational state.
-                  properties:
-                    lastTransitionTime:
-                      description: Last time the condition transitioned from one status
-                        to another. This should be when the underlying condition changed.
-                        If that is not known, then using the time when the API field
-                        changed is acceptable.
-                      format: date-time
-                      type: string
-                    message:
-                      description: A human readable message indicating details about
-                        the transition. This field may be empty.
-                      type: string
-                    reason:
-                      description: The reason for the condition's last transition
-                        in CamelCase. The specific API may choose whether or not this
-                        field is considered a guaranteed API. This field may not be
-                        empty.
-                      type: string
-                    severity:
-                      description: Severity provides an explicit classification of
-                        Reason code, so the users or machines can immediately understand
-                        the current situation and act accordingly. The Severity field
-                        MUST be set only when Status=False.
-                      type: string
-                    status:
-                      description: Status of the condition, one of True, False, Unknown.
-                      type: string
-                    type:
-                      description: Type of condition in CamelCase or in foo.example.com/CamelCase.
-                        Many .condition.type values are consistent across resources
-                        like Available, but because arbitrary conditions can be useful
-                        (see .node.status.conditions), the ability to deconflict is
-                        important.
-                      type: string
-                  required:
-                  - lastTransitionTime
-                  - status
-                  - type
-                  type: object
-                type: array
-            type: object
-        type: object
-    served: true
-    storage: false
-    subresources:
-      status: {}
 status:
   acceptedNames:
     kind: ""

I'm playing around with a few ideas, but nothing is working so far. Ideally we'd just mark apis/run as a Go module, but its v1alpha1 and v1alpha2 packages pull in all of the TF project's dependency graph.

@akutz
Copy link
Contributor Author

akutz commented Apr 22, 2022

So there is a work-around, but it reformats the CRDs in such a way that they look changed, but only due to formatting. There's a command for controller-gen called schemapatch, ex.:

	# Generate OpenAPI v3 schemas for API packages and merge them into existing CRD manifests
	controller-gen schemapatch:manifests=./manifests output:dir=./manifests paths=./pkg/apis/... 

Using this we can:

  1. Generate CRDs from the "root" module normally
  2. Generate the CRDs for the "child" module and patch the results into the CRDs from the first step

Again, the result of the second step produces CRDs that are formatted differently (indentation, order, etc.), but have the same contents.

Update
Apparently the reason the output from the schemapatch command is different is because that command uses different logic for marshaling the data to YAML:

Basically these two commands emit the calculated objects to YAML with entirely different logic, resulting in entirely inconsistent output.

I think it's possible for either to use the marshal logic of the other, but it's unclear to me if that's desired. For example, the disparate behavior could be by design.

@akutz
Copy link
Contributor Author

akutz commented Apr 23, 2022

I filed a patch for controller-tools that introduces cross-module support when generating CRDs, webhooks, RBAC rules, etc. -- kubernetes-sigs/controller-tools#663. I verified this patch by building controller-gen and validating the missing CRDs from this PR are now present.

@marckhouzam
Copy link
Contributor

@akutz I believe this has been addressed by @vijaykatam in #3169.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants