Skip to content

GA: Promote Receiver API to notification.toolkit.fluxcd.io/v1#498

Merged
makkes merged 4 commits intomainfrom
receiver-v1
Mar 30, 2023
Merged

GA: Promote Receiver API to notification.toolkit.fluxcd.io/v1#498
makkes merged 4 commits intomainfrom
receiver-v1

Conversation

@makkes
Copy link
Copy Markdown
Member

@makkes makkes commented Mar 22, 2023

API changes

The Receiver kind was promoted from v1beta2 to v1 (GA). All other kinds of the notification.toolkit.fluxcd.io group stay at version v1beta2.

The receivers.notification.toolkit.fluxcd.io CRD contains the following versions:

  • v1 (storage version)
  • v1beta2 (deprecated)
  • v1beta1 (deprecated)

Upgrade

The Receiver v1 API is backwards compatible with v1beta2.

To upgrade from v1beta2, after deploying the new CRD and controller, set apiVersion: notification.toolkit.fluxcd.io/v1 in the YAML files that contain Receiver definitions. Bumping the API version in manifests can be done gradually. It is advised to not delay this procedure as the beta versions will be removed after 6 months.

refs #436

Tasks tbd before merging:

  • update SOURCE_VER in Makefile to v1.0.0
  • squash commits into reasonable groups

@makkes makkes force-pushed the receiver-v1 branch 2 times, most recently from 8611b77 to 1e6bd1e Compare March 23, 2023 12:29
@stefanprodan stefanprodan changed the title Introduce v1 API and bump Receiver version to v1 GA: Promote Receiver API to notification.toolkit.fluxcd.io/v1 Mar 23, 2023
@stefanprodan
Copy link
Copy Markdown
Member

@makkes please fix the "the object has been modified; please apply your changes to the latest version and try again", we need to reload the objects in tests before an update.

makkes pushed a commit to fluxcd/website that referenced this pull request Mar 23, 2023
With fluxcd/notification-controller#498 we're graduating the Receiver
API group version to v1 and keeping all other kinds' version at
v1beta2. Therefore we need to publish API docs for both version.

Signed-off-by: Max Jonas Werner <mail@makk.es>
makkes pushed a commit to fluxcd/website that referenced this pull request Mar 23, 2023
With fluxcd/notification-controller#498 we're graduating the Receiver
API group version to v1 and keeping all other kinds' version at
v1beta2. Therefore we need to publish API docs for both version.

refs fluxcd/notification-controller#436

Signed-off-by: Max Jonas Werner <mail@makk.es>
makkes pushed a commit to fluxcd/website that referenced this pull request Mar 23, 2023
With fluxcd/notification-controller#498 we're graduating the Receiver
API group version to v1 and keeping all other kinds' version at
v1beta2. Therefore we need to publish API docs for both version.

refs fluxcd/notification-controller#436

Signed-off-by: Max Jonas Werner <mail@makk.es>
makkes pushed a commit to fluxcd/website that referenced this pull request Mar 23, 2023
With fluxcd/notification-controller#498 we're graduating the Receiver
API group version to v1 and keeping all other kinds' version at
v1beta2. Therefore we need to publish API docs for both version.

refs fluxcd/notification-controller#436

Signed-off-by: Max Jonas Werner <mail@makk.es>
makkes pushed a commit to fluxcd/website that referenced this pull request Mar 23, 2023
With fluxcd/notification-controller#498 we're graduating the Receiver
API group version to v1 and keeping all other kinds' version at
v1beta2. Therefore we need to publish API docs for both version.

refs fluxcd/notification-controller#436

Signed-off-by: Max Jonas Werner <mail@makk.es>
makkes pushed a commit to fluxcd/website that referenced this pull request Mar 23, 2023
With fluxcd/notification-controller#498 we're graduating the Receiver
API group version to v1 and keeping all other kinds' version at
v1beta2. Therefore we need to publish API docs for both version.

In addition to that we want the API navigation entry to appear at the
bottom of the tree. To accomplish that the import script is now able
to parse a weight parameter from imported markdown specs and put it
into the front matter of the resulting file.

refs fluxcd/notification-controller#436

Signed-off-by: Max Jonas Werner <mail@makk.es>
makkes pushed a commit to fluxcd/website that referenced this pull request Mar 23, 2023
With fluxcd/notification-controller#498 we're graduating the Receiver
API group version to v1 and keeping all other kinds' version at
v1beta2. Therefore we need to publish API docs for both version.

In addition to that we want the API navigation entry to appear at the
bottom of the tree. To accomplish that the import script is now able
to parse a weight parameter from imported markdown specs and put it
into the front matter of the resulting file.

refs fluxcd/notification-controller#436

Signed-off-by: Max Jonas Werner <mail@makk.es>
@makkes makkes marked this pull request as ready for review March 23, 2023 16:34
@makkes makkes requested review from a team and stefanprodan March 23, 2023 16:35
Comment thread api/v1/doc.go Outdated
@makkes makkes added this to the Bootstrap GA milestone Mar 23, 2023
Comment thread config/crd/bases/notification.toolkit.fluxcd.io_receivers.yaml Outdated
@makkes makkes requested a review from stefanprodan March 24, 2023 13:16
@makkes makkes force-pushed the receiver-v1 branch 2 times, most recently from 11adbe7 to 45cdae1 Compare March 27, 2023 16:32
@makkes
Copy link
Copy Markdown
Member Author

makkes commented Mar 28, 2023

Manual upgrade testing:

  1. Bootstrap Flux 0.41.2
  2. Put a Receiver v1beta2 manifest in Git and wait for it to be reconciled
  3. Build the n-c container image from this PR and make the image available to the cluster
  4. Put the Receiver CRD manifest from this PR into the bootstrap repo and patch the n-c Deployment to refer to the newly built image
  5. Push the changes and wait for the flux-system Kustomization to get ready and the new n-c Pod to be spun up
  6. Verify the Receiver object in the cluster is still ready
  7. Check the managed fields of the Receiver
  8. Update the Receiver manifest in Git to API version v1
  9. Verify the Receiver object in the cluster is still ready
  10. Check the managed fields of the Receiver
  11. Modify receiver object in Git, push and reconcile

Results:

  • After step 6, Receiver is still ready
  • Managed fields after CRD and n-c upgrade:
    managedFields:
    - apiVersion: notification.toolkit.fluxcd.io/v1beta2
      fieldsType: FieldsV1
      fieldsV1:
        f:metadata:
          f:labels:
            f:kustomize.toolkit.fluxcd.io/name: {}
            f:kustomize.toolkit.fluxcd.io/namespace: {}
        f:spec:
          f:events: {}
          f:resources: {}
          f:secretRef:
            f:name: {}
          f:type: {}
      manager: kustomize-controller
      operation: Apply
      time: "2023-03-28T10:30:33Z"
    - apiVersion: notification.toolkit.fluxcd.io/v1beta2
      fieldsType: FieldsV1
      fieldsV1:
        f:metadata:
          f:finalizers:
            .: {}
            v:"finalizers.fluxcd.io": {}
      manager: notification-controller
      operation: Update
      time: "2023-03-28T10:29:40Z"
    - apiVersion: notification.toolkit.fluxcd.io/v1beta2
      fieldsType: FieldsV1
      fieldsV1:
        f:status:
          f:conditions: {}
          f:observedGeneration: {}
          f:url: {}
          f:webhookPath: {}
      manager: notification-controller
      operation: Update
      subresource: status
      time: "2023-03-28T10:30:33Z"
    
  • Managed fields after step 10:
    
    managedFields:
    - apiVersion: notification.toolkit.fluxcd.io/v1beta2
      fieldsType: FieldsV1
      fieldsV1:
        f:metadata:
          f:labels:
            f:kustomize.toolkit.fluxcd.io/name: {}
            f:kustomize.toolkit.fluxcd.io/namespace: {}
        f:spec:
          f:events: {}
          f:resources: {}
          f:secretRef:
            f:name: {}
          f:type: {}
      manager: kustomize-controller
      operation: Apply
      time: "2023-03-28T10:30:33Z"
    - apiVersion: notification.toolkit.fluxcd.io/v1beta2
      fieldsType: FieldsV1
      fieldsV1:
        f:metadata:
          f:finalizers:
            .: {}
            v:"finalizers.fluxcd.io": {}
      manager: notification-controller
      operation: Update
      time: "2023-03-28T10:29:40Z"
    - apiVersion: notification.toolkit.fluxcd.io/v1beta2
      fieldsType: FieldsV1
      fieldsV1:
        f:status:
          f:conditions: {}
          f:observedGeneration: {}
          f:url: {}
          f:webhookPath: {}
      manager: notification-controller
      operation: Update
      subresource: status
      time: "2023-03-28T10:30:33Z"
    - apiVersion: notification.toolkit.fluxcd.io/v1beta2
      fieldsType: FieldsV1
      fieldsV1:
        f:metadata:
          f:annotations:
            .: {}
            f:reconcile.fluxcd.io/requestedAt: {}
      manager: flux
      operation: Update
      time: "2023-03-28T10:54:08Z"
    - apiVersion: notification.toolkit.fluxcd.io/v1
      fieldsType: FieldsV1
      fieldsV1:
        f:status:
          f:lastHandledReconcileAt: {}
      manager: notification-controller
      operation: Update
      subresource: status
      time: "2023-03-28T10:54:08Z"
    
  • After step 11:
    
    managedFields:
    - apiVersion: notification.toolkit.fluxcd.io/v1
      fieldsType: FieldsV1
      fieldsV1:
        f:metadata:
          f:labels:
            f:kustomize.toolkit.fluxcd.io/name: {}
            f:kustomize.toolkit.fluxcd.io/namespace: {}
        f:spec:
          f:events: {}
          f:resources: {}
          f:secretRef:
            f:name: {}
          f:type: {}
      manager: kustomize-controller
      operation: Apply
      time: "2023-03-28T10:59:18Z"
    - apiVersion: notification.toolkit.fluxcd.io/v1beta2
      fieldsType: FieldsV1
      fieldsV1:
        f:metadata:
          f:finalizers:
            .: {}
            v:"finalizers.fluxcd.io": {}
      manager: notification-controller
      operation: Update
      time: "2023-03-28T10:29:40Z"
    - apiVersion: notification.toolkit.fluxcd.io/v1beta2
      fieldsType: FieldsV1
      fieldsV1:
        f:status:
          f:url: {}
          f:webhookPath: {}
      manager: notification-controller
      operation: Update
      subresource: status
      time: "2023-03-28T10:30:33Z"
    - apiVersion: notification.toolkit.fluxcd.io/v1beta2
      fieldsType: FieldsV1
      fieldsV1:
        f:metadata:
          f:annotations:
            .: {}
            f:reconcile.fluxcd.io/requestedAt: {}
      manager: flux
      operation: Update
      time: "2023-03-28T10:54:08Z"
    - apiVersion: notification.toolkit.fluxcd.io/v1
      fieldsType: FieldsV1
      fieldsV1:
        f:status:
          f:conditions: {}
          f:lastHandledReconcileAt: {}
          f:observedGeneration: {}
      manager: notification-controller
      operation: Update
      subresource: status
      time: "2023-03-28T10:59:18Z"
    

Comment thread docs/spec/v1beta2/events.md
Comment thread api/v1/receiver_types.go Outdated
Comment thread api/v1/receiver_types.go
Comment thread api/v1/reference_types.go Outdated
Comment thread controllers/receiver_controller.go Outdated
Comment thread docs/spec/v1/receivers.md Outdated
Comment thread docs/spec/v1/receivers.md Outdated
Comment thread docs/spec/v1/receivers.md
Comment thread docs/spec/v1/receivers.md Outdated
@makkes
Copy link
Copy Markdown
Member Author

makkes commented Mar 29, 2023

@darkowlzz @somtochiama addressed all of your comments.

Copy link
Copy Markdown
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

Comment thread api/v1/receiver_types.go Outdated
@makkes
Copy link
Copy Markdown
Member Author

makkes commented Mar 29, 2023

We may need to update https://github.com/fluxcd/notification-controller/blob/main/docs/spec/README.md, similar to how it's updated in SC fluxcd/source-controller@d905985#diff-c0be2b6354cdd4c26a5c58880a73ec4206d21d8e4c67605ddf6c78fb89e56f90 .

I'm not sure I understand the motivation for removing the paragraphs from the README in s-c and the commit message doesn't mention any. Do we not want this documentation to be available anywhere? I don't really mind but would like to understand how to adapt the README here in n-c accordingly.

@makkes makkes force-pushed the receiver-v1 branch 2 times, most recently from 2259c84 to 249f0d1 Compare March 29, 2023 12:30
@darkowlzz
Copy link
Copy Markdown
Contributor

darkowlzz commented Mar 29, 2023

Do we not want this documentation to be available anywhere?

@makkes I don't know the historic reason for that document but after reading them, it seems like they were initial design docs for the components and now after some maturity and development, most of the specifications of the components are present in the individual API spec docs. So, maybe that document can just link to the spec docs of different versions like in the case of SC.

@stefanprodan
Copy link
Copy Markdown
Member

@makkes can you please redo the PR description to document the changes and upgrade procedure? This will help with the changelog.

Examples:

@darkowlzz
Copy link
Copy Markdown
Contributor

darkowlzz commented Mar 29, 2023

internal/server/receiver_server.go is still using the v1beta2 API.

@darkowlzz
Copy link
Copy Markdown
Contributor

darkowlzz commented Mar 29, 2023

v1beta2 CrossNamespaceObjectReference is used by EventServer in internal/server/event_handlers.go. Similar to how in SC all the non-v1 objects refer to the v1 Artifact, I think we can do the same here and use v1 CrossNamespaceObjectReference.

@makkes
Copy link
Copy Markdown
Member Author

makkes commented Mar 29, 2023

v1beta2 CrossNamespaceObjectReference is used by EventServer in internal/server/event_handlers.go. Similar to how in SC all the non-v1 objects refer to the v1 Artifact, I think we can do the same here and use v1 CrossNamespaceObjectReference.

The CrossNamespaceObjectReference used in EventServer is directly consumed from the Alert spec, that's why it's a v1beta2 reference, not a v1 one.

@darkowlzz
Copy link
Copy Markdown
Contributor

darkowlzz commented Mar 29, 2023

Based on the last comment, all the API definitions in api/v1beta2 that have reference to CrossNamespaceObjectReference should import v1 API and refer to it, similar to how it's done for all the Artifact references in SC v1beta2 API.

@makkes
Copy link
Copy Markdown
Member Author

makkes commented Mar 29, 2023

Based on the last comment, all the API definitions in api/v1beta2 that have reference to CrossNamespaceObjectReference should import v1 API and refer to it, similar to how it's done for all the Artifact references in SC v1beta2 API.

I must admit I find it strange to not confine all v1beta2 types to their respective Go package but rather reach out to definitions in another API version. This puts a lot of cognitive burden on developers that are working with the v1 types because they might be used in older versions of the API.

@stefanprodan
Copy link
Copy Markdown
Member

stefanprodan commented Mar 29, 2023

I must admit I find it strange to not confine all v1beta2 types to their respective Go package but rather reach out to definitions in another API version. This puts a lot of cognitive burden on developers that are working with the v1 types because they might be used in older versions of the API.

It's only one package github.com/notification-controller/api. So the moment you update the package, all the API versions get imported, the same way it works for Kubernetes apimachinery. What we are doing is similar to Kubernetes Deployments back when Pod Spec was at v1 and Deployments were beta, the beta structs were using the v1 types.

@darkowlzz
Copy link
Copy Markdown
Contributor

darkowlzz commented Mar 30, 2023

make api-docs shows the following warnings:

W0330 13:36:30.098587 3756196 main.go:445] not found external link source for type github.com/fluxcd/notification-controller/api/v1.CrossNamespaceObjectReference
W0330 13:36:30.099848 3756196 main.go:445] not found external link source for type github.com/fluxcd/notification-controller/api/v1.CrossNamespaceObjectReference
W0330 13:36:30.100374 3756196 main.go:445] not found external link source for type github.com/fluxcd/notification-controller/api/v1.CrossNamespaceObjectReference
W0330 13:36:30.102066 3756196 main.go:445] not found external link source for type github.com/fluxcd/notification-controller/api/v1.CrossNamespaceObjectReference

Let's update hack/api-docs/config.json and regenerate docs similar to what's done in SC fluxcd/source-controller@8fcfde9#diff-739b83c5f1e9b3281f7174a89731ce41944600811270b18f67e5a7d97289d262 .

@makkes makkes force-pushed the receiver-v1 branch 3 times, most recently from 08b1ae2 to af686a3 Compare March 30, 2023 09:23
Comment thread api/v1/condition_types.go
Comment thread docs/spec/v1/README.md Outdated
Copy link
Copy Markdown
Contributor

@darkowlzz darkowlzz left a comment

Choose a reason for hiding this comment

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

LGTM!

@stefanprodan stefanprodan added the hold Issues and pull requests put on hold label Mar 30, 2023
Copy link
Copy Markdown
Member

@stefanprodan stefanprodan left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks @makkes and @darkowlzz

PS. We need to wait for SC release, update the API refs in here, then merge.

Max Jonas Werner added 4 commits March 30, 2023 15:40
This commit bumps the Receiver API version to v1 in preparation of
the Flux GitOps GA milestone
(https://fluxcd.io/roadmap/#flux-gitops-ga-q1-2023).

We are now actively maintaining two versions of the notification API
group in parallel: v1 which currently only holds the Receiver kind and
v1beta2 for all other kinds.

Since we haven't run into this situation before, I had to change the
way we expose the API docs in ./docs/api: The directory now has
sub-directories for each active API version. Therefore we need to
change our scripts in the website repository to take this change into
account so that we expose both API group version at
https://fluxcd.io/flux/components/notification/api/. This change is
implemented in fluxcd/website#1427.

refs #436

Signed-off-by: Max Jonas Werner <mail@makk.es>
This functionality has been implemented in #482 but we only want to
expose it in v1 of the API.

Signed-off-by: Max Jonas Werner <mail@makk.es>
Signed-off-by: Max Jonas Werner <mail@makk.es>
v1 will be the default version in the upcoming version of n-c.

Signed-off-by: Max Jonas Werner <mail@makk.es>
@makkes makkes removed the hold Issues and pull requests put on hold label Mar 30, 2023
@makkes makkes merged commit 0af806c into main Mar 30, 2023
@makkes makkes deleted the receiver-v1 branch March 30, 2023 14:22
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.

4 participants