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

Enable Server-Side Apply mode by default #2206

Merged
merged 4 commits into from
Oct 19, 2022

Conversation

lblackstone
Copy link
Member

Proposed changes

Related issues (optional)

@github-actions

This comment was marked as outdated.

Copy link
Contributor

@squaremo squaremo left a comment

Choose a reason for hiding this comment

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

Does the comment in provider.go starting L2138 need to be updated, now the default is server-side apply? It says

In the next major release, we will default to using Server-side Apply, which will simplify this logic.

provider/pkg/provider/provider.go Outdated Show resolved Hide resolved
provider/pkg/gen/schema.go Outdated Show resolved Hide resolved
@lblackstone
Copy link
Member Author

Does the comment in provider.go starting L2138 need to be updated, now the default is server-side apply? It says

In the next major release, we will default to using Server-side Apply, which will simplify this logic.

We plan to drop the provider flag in v4, but may need to maintain some of the existing client-side logic for previews, etc. This would be an implementation detail rather than an opt-in, though. I'll leave this comment for now.

@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@github-actions

This comment was marked as outdated.

This test was accessing an Output property without using an apply, and the value had not resolved already. Apparently this was working with the legacy Client-Side Apply implementation, but the test code was unsafe.
@github-actions

This comment was marked as outdated.

With Server-side Apply previews, updating an immutable field in a resource will result in a 422 error code from the API server. In these cases, Pulumi will replace the resource, so we need to ignore the error rather than failing the preview.
@github-actions

This comment was marked as outdated.

1 similar comment
@github-actions
Copy link

Does the PR have any schema changes?

Looking good! No breaking changes found.
No new resources/functions.

@lblackstone lblackstone marked this pull request as ready for review October 19, 2022 15:58
@lblackstone lblackstone merged commit 5960e58 into master Oct 19, 2022
@pulumi-bot pulumi-bot deleted the lblackstone/flip-ssa-default branch October 19, 2022 23:14
@pstovik
Copy link

pstovik commented Oct 24, 2022

Hi @lblackstone
after updating to the latest = 3.22.0 version of "@pulumi/kubernetes" npm package we are getting following kind of error running "pulumi preview"

"message": "error: Preview failed: 1 error occurred:\n\t* the Kubernetes API server reported that "cert-manager-webhook" failed to fully initialize or become live: use pulumi.com/patchForce to override the conflict: Apply failed with 2 conflicts: conflicts with

what should be the proper approach to solve the issue and become using the SSA?
(now: we are gone to mitigate the issue by using "pulumi provider level" configuration with {enableServerSideApply: false})

Best regards Petr

context:

AKS cluster version 1.23.8 (running since 2021-03-05)
pulumi CLI version 3.40.2

the kubernetes resource has following definition:

# (result of running `kubectl describe validatingwebhookconfiguration.admissionregistration.k8s.io/cert-manager-webhook`)
Name:         cert-manager-webhook
Namespace:
Labels:       app=webhook
              app.kubernetes.io/component=webhook
              app.kubernetes.io/instance=cert-manager
              app.kubernetes.io/managed-by=pulumi
              app.kubernetes.io/name=webhook
              app.kubernetes.io/version=v1.9.1
Annotations:  cert-manager.io/inject-ca-from-secret: cert-manager/cert-manager-webhook-ca
API Version:  admissionregistration.k8s.io/v1
Kind:         ValidatingWebhookConfiguration
Metadata:
  Creation Timestamp:  2021-03-05T07:58:36Z
  Generation:          11
  Managed Fields:
    API Version:  admissionregistration.k8s.io/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:webhooks:
        k:{"name":"webhook.cert-manager.io"}:
          f:clientConfig:
            f:caBundle:
    Manager:      cainjector
    Operation:    Update
    Time:         2021-11-03T15:58:56Z
    API Version:  admissionregistration.k8s.io/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:webhooks:
        k:{"name":"webhook.cert-manager.io"}:
          f:namespaceSelector:
    Manager:      admissionsenforcer
    Operation:    Update
    Time:         2022-09-12T07:36:11Z
    API Version:  admissionregistration.k8s.io/v1
    Fields Type:  FieldsV1
    fieldsV1:
      f:metadata:
        f:annotations:
          .:
          f:cert-manager.io/inject-ca-from-secret:
          f:kubectl.kubernetes.io/last-applied-configuration:
        f:labels:
          .:
          f:app:
          f:app.kubernetes.io/component:
          f:app.kubernetes.io/instance:
          f:app.kubernetes.io/managed-by:
          f:app.kubernetes.io/name:
          f:app.kubernetes.io/version:
      f:webhooks:
        .:
        k:{"name":"webhook.cert-manager.io"}:
          .:
          f:admissionReviewVersions:
          f:clientConfig:
            .:
            f:service:
              .:
              f:name:
              f:namespace:
              f:path:
              f:port:
          f:failurePolicy:
          f:matchPolicy:
          f:name:
          f:objectSelector:
          f:rules:
          f:sideEffects:
          f:timeoutSeconds:
    Manager:         pulumi-resource-kubernetes
    Operation:       Update
    Time:            2022-09-12T07:36:11Z
  Resource Version:  321093113
  UID:               07fc97cb-686d-4deb-b1d5-d75b116fa4e6
Webhooks:
  Admission Review Versions:
    v1
  Client Config:
    Ca Bundle:  ... /*INTENTIONALLY REMOVED*/ ...
    Service:
      Name:        cert-manager-webhook
      Namespace:   cert-manager
      Path:        /validate
      Port:        443
  Failure Policy:  Fail
  Match Policy:    Equivalent
  Name:            webhook.cert-manager.io
  Namespace Selector:
    Match Expressions:
      Key:       cert-manager.io/disable-validation
      Operator:  NotIn
      Values:
        true
      Key:       name
      Operator:  NotIn
      Values:
        cert-manager
      Key:       control-plane
      Operator:  DoesNotExist
  Object Selector:
  Rules:
    API Groups:
      cert-manager.io
      acme.cert-manager.io
    API Versions:
      v1
    Operations:
      CREATE
      UPDATE
    Resources:
      */*
    Scope:          *
  Side Effects:     None
  Timeout Seconds:  10
Events:             <none>

@dotansimha
Copy link

"message": "error: Preview failed: 1 error occurred:\n\t* the Kubernetes API server reported that "cert-manager-webhook" failed to fully initialize or become live: use pulumi.com/patchForce to override the conflict: Apply failed with 2 conflicts: conflicts with

Same here 👍 we are getting this with cert-manager deployment:

kubernetes:admissionregistration.k8s.io/v1:ValidatingWebhookConfiguration (cert-manager-webhook):
      error: 1 error occurred:
      	* the Kubernetes API server reported that "cert-manager-webhook" failed to fully initialize or become live: use `pulumi.com/patchForce` to override the conflict: Apply failed with 2 conflicts: conflicts with "admissionsenforcer" using admissionregistration.k8s.io/v1:
      - .webhooks[name="webhook.cert-manager.io"].namespaceSelector
      conflicts with "pulumi-resource-kubernetes" using admissionregistration.k8s.io/v1:
      - .webhooks[name="webhook.cert-manager.io"].rules
      - ```

lblackstone added a commit that referenced this pull request Oct 27, 2022
This reverts commit 5960e58.

# Conflicts:
#	CHANGELOG.md
lblackstone added a commit that referenced this pull request Oct 27, 2022
@squaremo
Copy link
Contributor

@pstovik @dotansimha Sorry about that folks. SSA being on by default was reverted in v3.22.1, until we can clear up problems like those you reported. Bumping the package version will get the new provider (e.g., npm update @pulumi/kubernetes).

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.

None yet

5 participants