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

fix: Update the k8s Job container logic for custom actions to match v… #9584

Merged
merged 4 commits into from
Nov 25, 2024

Conversation

mattsanta
Copy link
Contributor

@mattsanta mattsanta commented Nov 25, 2024

…erify

Fixes #9409

Description

This PR updates the logic when it comes to custom actions with a provided Job manifest to be similar to verify. What this means is that any container fields defined in the Job manifest that are not configurable in the customActions stanza are kept, e.g. volumes. It also appends env vars rather than replacing to be consistent with verify as well.

Frankly, I'm not sure why the Job execution for verify and custom actions were implemented in two different places which caused these deviations. I suspect there are other ways in which they differ, just from a quick glance at the two implementations.

Unfortunately, it also looks like executing the k8s Jobs for verify and custom actions lacks unit testing. In order to get this in I've just gone ahead and added a very basic test for the container patching logic, same as what exists for verify here.

User facing changes (remove if N/A)

Any fields that are defined in the Job container manifest that are not configurable in the customActions container stanza will be kept. In other words, any fields that are not name, image, command, or args.

@mattsanta mattsanta merged commit fae5fb4 into GoogleContainerTools:main Nov 25, 2024
11 of 12 checks passed
@cmanzi-bestow
Copy link

@mattsanta Thanks for addressing this! Has this change been rolled out to GCP (Cloud Deploy)?

@rhhammond
Copy link

rhhammond commented Jan 3, 2025

@cmanzi-bestow it looks like CloudDeploy is using v2.13.x by default, and v2.13.2 was released in August.

@mattsanta @louisjimenez is there a release roadmap for skaffold that we can follow? Will this change be in v2.13.3 in the near future, or require a new minor version? It looks like the official versioning policy is that CloudDeploy picks a new version of skaffold every 90 days.

@plumpy
Copy link
Collaborator

plumpy commented Jan 10, 2025

We're working to release a v2.14 soon, and then we'll add it to Cloud Deploy within a few weeks after that. Sorry for the delay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom actions ignores container volumes
5 participants