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

chore: new schema version v4beta13 #9690

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

reingart
Copy link

@reingart reingart commented Jan 29, 2025

Signed-off-by: Mariano Reingart <[email protected]>
@reingart reingart requested a review from a team as a code owner January 29, 2025 05:36
@reingart reingart requested a review from katiexzhang January 29, 2025 05:36
Copy link

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

Hi team,

I'm currently reviewing this pull request and will provide feedback shortly. In the meantime, here's a summary of the changes for you and other reviewers to quickly get up to speed:

Pull Request Summary:

  • Intent: This PR updates the Skaffold schema version to v4beta13. This appears to be a routine update based on a previous GitHub Actions run.
  • Changes:
    • The skaffold_version in docs-v2/config.toml was updated from skaffold/v4beta12 to skaffold/v4beta13 (lines 82-88).
    • The apiVersion in numerous example skaffold.yaml files under the integration/examples directory was updated from skaffold/v4beta12 to skaffold/v4beta13 (lines 1-4 in each affected file). This includes files for various build systems and deployment methods.
    • Similar changes were made to the apiVersion in many files under the integration/testdata directory (lines 1-4 in each affected file).
    • The apiVersion in pkg/skaffold/schema/latest/config.go was updated from skaffold/v4beta12 to skaffold/v4beta13 (lines 25-32).
    • Upgrade paths were updated in pkg/skaffold/schema/v4beta11/upgrade.go and pkg/skaffold/schema/v4beta11/upgrade_test.go to reflect the new version (lines 17-23 in each file).
    • The Version constant in pkg/skaffold/schema/v4beta12/config.go remains as skaffold/v4beta12 (lines 28-29), which is expected as this file pertains to the previous version.
    • The versions.go file was updated to include the new v4beta12 schema version (lines 93 and 186-190).
  • Activity: The PR description simply references a GitHub Actions run that triggered the need for this version bump.

Note: The changes are primarily version updates, and the core functionality of Skaffold remains unchanged.


And now, a little verse to lighten the mood:

Schema's changed, a new release,
Version bump, a simple peace,
Code flows, smooth and free.

Copy link

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

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

This pull request updates the Skaffold schema version to v4beta13. The change looks good and correctly updates all example files and testdata.
I've summarized the style guide elements I referenced below:
No style guide was provided, so I defaulted to commonly used style guides per language (e.g. for YAML files, I referenced the Kubernetes style guide).


// Upgrade upgrades a configuration to the next version.
// Config changes from v4beta12 to v4beta13
func (c *SkaffoldConfig) Upgrade() (util.VersionedConfig, error) {
Copy link

@katiexzhang katiexzhang Jan 31, 2025

Choose a reason for hiding this comment

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

Hi @reingart thanks for the PR. Just confirming, this file and the test file got generated from running hack/new-version.sh?

Copy link
Author

Choose a reason for hiding this comment

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

yes, I run ./hack/new-version.sh following the instructions from docs and GHA messages:

RUN hack/check-schema-changes.sh
time="2025-01-28T13:41:40Z" level=warning msg="Detected changes to the latest config. Checking on Github if it's released..."
time="2025-01-28T13:41:40Z" level=info msg="Checking for released status of v4beta12..."
time="2025-01-28T13:41:40Z" level=info msg="last release tag: v2.14.0"
time="2025-01-28T13:41:40Z" level=info msg="Last released version: v4beta12"

What should I do?
-----------------
 + If this retroactive change is required and is harmless to users, indicate on your PR.
 + Check if a new unreleased version has been created:
     - Ensure that your branch is up-to-date with the "origin/main" branch.
     - Check for a pending PR to create a new version.
 + Create a separate PR with just the result of running the 'hack/new-version.sh' script.

Invalid changes:
----------------
time="2025-01-28T13:41:40Z" level=error msg="Schema \"v4beta12\" is already released. Changing it is not allowed."
time="2025-01-28T13:41:40Z" level=fatal msg="structural changes detected"
diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go
index a4e802[6](https://github.com/GoogleContainerTools/skaffold/actions/runs/12860696016/job/36290656644?pr=9648#step:6:7)c9..01da161cb 100644
--- a/pkg/skaffold/schema/latest/config.go
+++ b/pkg/skaffold/schema/latest/config.go
@@ -25,7 +25,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/v2/pkg/skaffold/schema/util"
 )
 
-// !!! WARNING !!! This config version is already released, please DO NOT MODIFY the structs in this file.
+// This config version is not yet released, it is SAFE TO MODIFY the structs in this file.
 const Version string = "skaffold/v4beta[12](https://github.com/GoogleContainerTools/skaffold/actions/runs/12860696016/job/36290656644?pr=9648#step:6:13)"

Then I have to run ./hack/generate-man.sh and ./hack/generate-man.sh to fix more issues reported iin the actions of this PR.
Itried it again:

vagrant@ubuntu-focal:/src$ git checkout -b newver
Switched to a new branch 'newver'
vagrant@ubuntu-focal:/src$ ls pkg/skaffold/schema/v4beta12/upgrade.go
ls: cannot access 'pkg/skaffold/schema/v4beta12/upgrade.go': No such file or directory
vagrant@ubuntu-focal:/src$ make clean
rm -rf ./out hack/bin fs/assets/check.txt fs/assets/schemas_generated/
vagrant@ubuntu-focal:/src$ ./hack/new-version.sh
INFO[0000] Previous Skaffold version: v4beta11          
DEBU[0000] Current Skaffold version: v4beta12           
Please enter new version (default: v4beta13): 
INFO[0009] Next Skaffold version: v4beta13              
./hack/new-version.sh: line 21: goimports: command not found
vagrant@ubuntu-focal:/src$ go install golang.org/x/tools/cmd/goimports@latest
vagrant@ubuntu-focal:/src$ ./hack/new-version.sh
INFO[0000] Previous Skaffold version: v4beta12          
DEBU[0000] Current Skaffold version: v4beta13           
INFO[0000] Checking for released status of v4beta13...  
INFO[0000] last release tag: v2.14.0                    
INFO[0000] Last released version: v4beta12              
FATA[0000] There is no need to create a new version, v4beta13 is still not released 
exit status 1
vagrant@ubuntu-focal:/src$ ls pkg/skaffold/schema/v4beta12/upgrade.go
pkg/skaffold/schema/v4beta12/upgrade.go

@reingart
Copy link
Author

reingart commented Feb 3, 2025

I had run generate schemas makkefile target, as instructed by the failing actions, but there are no changes to the codebase:

vagrant@ubuntu-focal:/src$ make generate-schemas-v2
go run hack/schemas/main.go
vagrant@ubuntu-focal:/src$ git diff
vagrant@ubuntu-focal:/src$ make generate-schemas
go run hack/schemas/main.go
vagrant@ubuntu-focal:/src$ git diff
vagrant@ubuntu-focal:/src$ git status
Refresh index: 100% (14709/14709), done.
On branch v4beta13
Untracked files:
  (use "git add <file>..." to include in what will be committed)
	config1077315267.v2
	config2125658384.v2

nothing added to commit but untracked files present (use "git add" to track)

There are helm unit tests failing, but I see them reverted in the main branch, so merged those latest changes.
After that, there were minor changes when running generate schemas (commited)

Also there are other errors link ERROR: failed to write buildpack group: open /layers/group.toml: permission denied, not sure if they are related to this PR (probably not I guess)

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.

2 participants