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

Move deliverable into test only path #133

Merged
merged 4 commits into from
Sep 22, 2021
Merged

Move deliverable into test only path #133

merged 4 commits into from
Sep 22, 2021

Conversation

emmjohnson
Copy link
Contributor

Closes #130

Co-authored-by: Sam Coward scoward@vmware.com

@@ -58,7 +58,7 @@ var _ = Describe("Registrar", func() {
Group: "carto.run",
Version: "v1alpha1",
}
Expect(len(scheme.KnownTypes(gv))).To(Equal(25))
Expect(len(scheme.KnownTypes(gv))).To(Equal(23))
Copy link
Contributor

Choose a reason for hiding this comment

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

at first I thought 'oh, 23? shouldn't it be 24?', but I guess the deduction of 2 is due to *List being registered 👍

@cirocosta
Copy link
Contributor

cirocosta commented Sep 21, 2021

dang, I just realized that we actually have Deliverable mentions in our site:

site/content/docs/install.md
47:^                    deliverables.carto.run             CustomResourceDefinition  -       -    create  -       reconcile  -   -

site/content/docs/about.md
14:* Deliverable ([Spec Reference](reference.md#deliverable))
16:The supply chain allows users to define the steps that an application must pass through in order to create an image. The deliverable portion of a Cartographer workflow allows the user to define the steps that the built image must pass through. By combining supply chains with delivery, the entire path to production can be specified and thereby choreographed by Cartographer.
20:Both the supply chain and delivery consist of components that are specified via Templates. Each template acts as a wrapper for existing Kubernetes resources and allows them to be used with Cartographer. There are currently four different types of templates that can be use in a Cartographer supply chain or deliverable:

site/content/docs/reference.md
143:  # deliverable state. (required, at least 1)

I think it'd be good to make sure we get rid of that

@@ -18,6 +18,8 @@ startControlPlane: true
testDirs:
- tests/kuttl
crdDir: config/crd/bases
manifestDirs:
- tests/crd
Copy link
Contributor

Choose a reason for hiding this comment

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

so, my understanding is that we're doing a plain move of the CustomResourceDefinition object that has been generated before to that directory, and then just submitting to Kubernetes when running kuttl, but there's no go-based definition of the struct that controller-gen would generate, right?

if that's the case, I'd say it'd be good if we could strip down the CustomResourceDefinition to a minimum, removing those descriptions like

        openAPIV3Schema:
          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

and the annotation that indicates that this was generated:

  metadata:
    annotations:
      controller-gen.kubebuilder.io/version: v0.6.2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not going to take the time to do this. I think we have some work to do around both "test" crds at the moment.

@cirocosta
Copy link
Contributor

oh btw, I just remembered that for the Test CRD used in integration we have

          SchemeGroupVersion = schema.GroupVersion{
                  Group:   "test.run",
                  Version: "v1alpha1",
          }

which imo is nice that allows one to quickly tell that such resource is not part of the main set of resources.

I think either here or in another PR we should move this object to such apiversion - wdyt?

emmjohnson and others added 2 commits September 22, 2021 10:30
Co-authored-by: Sam Coward <scoward@vmware.com>
[#130]

Co-authored-by: Emily Johnson <emjohnson@vmware.com>
@squeedee squeedee force-pushed the 130-move-deliverable branch from 77b82f0 to 71be641 Compare September 22, 2021 14:37
Rasheed Abdul-Aziz and others added 2 commits September 22, 2021 10:41
[#130]

Co-authored-by: Emily Johnson <emjohnson@vmware.com>
Co-authored-by: Rasheed Abdul-Aziz <rabdulaziz@vmware.com>
Copy link
Member

@squeedee squeedee left a comment

Choose a reason for hiding this comment

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

Good enough for beta!

@emmjohnson emmjohnson merged commit a3f8292 into main Sep 22, 2021
@emmjohnson emmjohnson deleted the 130-move-deliverable branch September 22, 2021 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove deliverable from crd, move it so it is available for tests
3 participants