feat: transform imagePullPolicy when using local cluster

[lucasrod16]

Relates to #6992

Description

Introduces a ReplaceImagePullPolicy function to update the imagePullPolicy field in Kubernetes manifests to Never when running on a local cluster. This prevents deployment errors that occur when imagePullPolicy is set to Always, as detailed in #6992.

User facing changes

Files to reproduce:

• Dockerfile

  FROM nginx:alpine

• pod.yaml

  apiVersion: v1
  kind: Pod
  metadata:
    name: nginx
  spec:
    containers:
    - name: nginx
      image: nginx:alpine
      imagePullPolicy: Always

• skaffold.yaml

  apiVersion: skaffold/v4beta11
  kind: Config
  metadata:
    name: nginx
  build:
    artifacts:
      - image: nginx
        context: .
        docker:
          dockerfile: Dockerfile
  manifests:
    rawYaml:
     - ./pod.yaml

Before:

After:

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[lucasrod16]

Hi @ericzzzzzzz,

I noticed that this pull request has been pending for a while. Is there anything needed from me? Please let me know if there are any updates required.

Thanks!

[ericzzzzzzz]

Hi, @lucasrod16 Thank you for the pr!

I think we should only do this transformation when using skaffold dev, skaffold run, skaffold debug as we know that the transformed manifests will be applied to the connected local cluster, but for skaffold render command, we're not sure how the manifests will be used, the rendered results may be used in other places even without skaffold.

Also, this pr does the requested change when kubectl deployer is used, it needs to do the similar thing when helm deployer is used.

[lucasrod16]

Hi, @lucasrod16 Thank you for the pr!

I think we should only do this transformation when using skaffold dev, skaffold run, skaffold debug as we know that the transformed manifests will be applied to the connected local cluster, but for skaffold render command, we're not sure how the manifests will be used, the rendered results may be used in other places even without skaffold.

Also, this pr does the requested change when kubectl deployer is used, it needs to do the similar thing when helm deployer is used.

@ericzzzzzzz That makes sense, thanks for the feedback!

I moved the transform logic from BaseTransform to kubectl Deploy and verified it works locally with dev, run, and debug.

I will work on adding the transform logic to the helm deployer as well

[alphanota]

Hello! Thank you for sending this PR! Please see my comments on it, thanks!.

Also for the PR description, since this PR doesn't have the helm changes needed to fully address the issue, I don't think we should close the bug when this PR is merged.

[alphanota]

This should be unexported, since this new struct is only used as an implementor of the ResourceSelector interface.

I would also add a comment about what this ResourceSelector does/ selects. For example
resourceSelectorImagePullPolicy is an implementation of ResourceSelector to select X and do Z.

[alphanota]

Should return the interface ResourceSelector instead of *ResourceSelectorImagePullPolicy.

[alphanota]

A short comment explaining the goal of the method and the criteria used would be helpful.

[alphanota]

Here too, a description of the goal would help.

[alphanota]

A comment explaining the purpose of this struct. ie imagePullPolicyReplacer implements FieldVisitor and is used to edit an entry in a YAML document.

[alphanota]

Please add a comment explaining this method also. For example Visit sets the value of the "imagePullPolicy" field in a Kubernetes manifest to "Never".

[mepi262]

@lucasrod16
The reviewer has commented to your pull request. How about check it?

[lucasrod16]

Hey @alphanota, thanks for the PR review! I've updated the PR description to ensure the related issue isn't closed when this PR is merged and have addressed all your feedback. I currently don't have the bandwidth to implement the helm functionality, and I appreciate your understanding. Thanks again for the feedback!