cluster-api-provider-openstack icon indicating copy to clipboard operation
cluster-api-provider-openstack copied to clipboard

🌱 add merge strategy markers

Open apricote opened this issue 3 years ago • 3 comments

What this PR does / why we need it:

This commit adds merge strategy markers to our v1alpha6 API. This is required for Server-Side Apply (SSA) to work correctly. SSA is used in Cluster API v1.2, and not having these markers might cause issues when using ClusterClass patches.

This only adds a subset of markers, where it was possible. I skipped adding markers for two cases:

  • We have many slices that do not have any proper map keys, as all fields are optional. This includes mostly "filter" structs, for example NetworkParam and SecurityGroupParam.
  • The selected map key may not be optional (omitempty). If it is, it can not be used as the map key. This applied to structs AddressPair and ExternalRouterIPParam.

Further documentation:

  • CAPI v1.2 provider migration: https://cluster-api.sigs.k8s.io/developer/providers/v1.1-to-v1.2.html#required-api-changes-for-providers
  • SSA Markers: https://kubernetes.io/docs/reference/using-api/server-side-apply/#merge-strategy

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged): Related to #1282

Special notes for your reviewer:

/hold

apricote avatar Aug 16 '22 12:08 apricote

Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name Link
Latest commit 8144c3994593be4be768e23bbf1254775e62f58c
Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/62fb933facf69b00094e5743
Deploy Preview https://deploy-preview-1325--kubernetes-sigs-cluster-api-openstack.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

netlify[bot] avatar Aug 16 '22 12:08 netlify[bot]

/approve /lgtm

Thanks for implementing the merge strategy markers 👍🏻 one step further towards CAPI v1.2

tobiasgiese avatar Aug 16 '22 18:08 tobiasgiese

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: apricote, tobiasgiese

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 /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot avatar Aug 16 '22 18:08 k8s-ci-robot

/lgtm /hold cancel

jichenjc avatar Aug 17 '22 00:08 jichenjc