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

[NET-9839] fix: add shared version submodule #4091

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

zalimeni
Copy link
Member

@zalimeni zalimeni commented Jun 7, 2024

Introduce a new submodule for sharing versioning code across the binary-producing submodules in consul-k8s repo.

This is both an improvement in terms of consolidation, as well as a fix for incorrect control-plane submodule pinning in the control-plane/cni submodule that led to incorrect dev versions being used in 1.4.2 and 1.4.3 (the first releases that included a valid version of control-plane/cni. See #4054 comments for more context.

I have a separate change (#4093) to move us to using an embedded VERSION plaintext file like consul, but splitting that out from this change for brevity and to reduce risk before the upcoming release. The follow-up is lower priority and may not be worth pushing through soon, but would be a nice simplification.

Once merged, I plan to manually backport this change to .x branches and release/1.5.0, modifying the versions as needed.

Changes proposed in this PR

  • Consolidate version management in consul-k8s binaries into a single shared (private) submodule
  • Fix a bug in prepare- scripts leaving chart version blank when version is non-dev
  • Unrelated cleanup in go.mod/go.sum due to running go mod tidy after module swaps

How I've tested this PR

control-plane Docker in CI:

Downloaded from CI run above:

~/Downloads/consul-k8s_1.6.0-dev_linux_amd64 
❯ docker run -it --rm --platform linux/amd64 -v "$(pwd):/home/test" -w "/home/test" ubuntu ./consul-k8s -version
v1.6.0-dev (cf59978+CHANGES)

~/Downloads/consul-cni_1.6.0-dev_linux_amd64   
❯ docker run -it --rm --platform linux/amd64 -v "$(pwd):/home/test" -w "/home/test" ubuntu ./consul-cni -version
CNI consul-cni plugin v1.6.0-dev (cf59978+CHANGES)
CNI protocol versions supported: 0.1.0, 0.2.0, 0.3.0, 0.3.1, 0.4.0, 1.0.0

Local:

❯ make version
1.6.0-dev

❯ cd control-plane/
~/workspace/consul-k8s/control-plane 
❯ go run . -version
v1.6.0-dev

❯ cd cni
~/workspace/consul-k8s/control-plane/cni 
❯ go run . -version
CNI consul-cni plugin v1.6.0-dev
CNI protocol versions supported: 0.1.0, 0.2.0, 0.3.0, 0.3.1, 0.4.0, 1.0.0

❯ cd ../../cli
~/workspace/consul-k8s/cli
❯ go run . -version
v1.6.0-dev

~/workspace/consul-k8s 
❯ make prepare-release
prepare_release: dir:/Users/michael.zalimeni/workspace/consul-k8s consul-k8s:1.999.1 consul:1.19.0 consul-dataplane:1.999.1 date:February 27, 2080 git tag:v1.4.3
==> Updating version/version.go with version info: 1.999.1
==> Updating Helm chart version, consul-k8s: 1.999.1  consul: 1.19.0  consul-dataplane: 1.999.1
Checking charts for hashicorpreview images. . .
Charts do not contain hashicorpreview images, ready for release!

❯ git status
On branch zalimeni/shared-version-cli-control-plane-cni
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   ../CHANGELOG.md
	modified:   ../charts/consul/Chart.yaml
	modified:   ../charts/consul/values.yaml
	modified:   ../version/version.go

❯ make version
1.999.1

❯ cd control-plane/
~/workspace/consul-k8s/control-plane !4  
❯ go run . -version
v1.999.1-dev

❯ cd cni
~/workspace/consul-k8s/control-plane/cni !4  
❯ go run . -version
CNI consul-cni plugin v1.999.1-dev
CNI protocol versions supported: 0.1.0, 0.2.0, 0.3.0, 0.3.1, 0.4.0, 1.0.0

❯ cd ../../cli
~/workspace/consul-k8s/cli !4
❯ go run . -version
v1.999.1-dev

## Emulate build steps in build.yml to remove dev markerexport GIT_COMMIT=$(git rev-parse --short HEAD)
export GIT_DIRTY=$(test -n "$(git status --porcelain)" && echo "+CHANGES")
export GIT_IMPORT=github.com/hashicorp/consul-k8s/version
export GOLDFLAGS="-X ${GIT_IMPORT}.GitCommit=${GIT_COMMIT}${GIT_DIRTY} -X ${GIT_IMPORT}.GitDescribe=1.999.1"cd control-plane/
~/workspace/consul-k8s/control-plane !1   
❯ go run -ldflags "${GOLDFLAGS}" . -version
v1.999.1

❯ cd cni
~/workspace/consul-k8s/control-plane/cni !1 
❯ go run -ldflags "${GOLDFLAGS}" . -version
CNI consul-cni plugin v1.999.1
CNI protocol versions supported: 0.1.0, 0.2.0, 0.3.0, 0.3.1, 0.4.0, 1.0.0

❯ cd ../../cli
~/workspace/consul-k8s/cli !1
❯ go run -ldflags "${GOLDFLAGS}" . -version
v1.999.1

How I expect reviewers to test this PR

👀 + 🧠

Checklist

@zalimeni zalimeni added the pr/no-backport signals that a PR will not contain a backport label label Jun 7, 2024
Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.
@zalimeni zalimeni force-pushed the zalimeni/shared-version-cli-control-plane-cni branch from 761b5bf to ad5dfaf Compare June 7, 2024 17:20
@zalimeni zalimeni force-pushed the zalimeni/shared-version-cli-control-plane-cni branch from c0c77f4 to c5cd1ab Compare June 7, 2024 17:57
@zalimeni
Copy link
Member Author

zalimeni commented Jun 7, 2024

make prepare-release test:
image

Copy link
Contributor

@DanStough DanStough left a comment

Choose a reason for hiding this comment

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

👍 Thank you for the fix!

@zalimeni zalimeni merged commit 053215c into main Jun 7, 2024
48 of 50 checks passed
@zalimeni zalimeni deleted the zalimeni/shared-version-cli-control-plane-cni branch June 7, 2024 19:56
zalimeni added a commit that referenced this pull request Jun 7, 2024
* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
zalimeni added a commit that referenced this pull request Jun 7, 2024
* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
zalimeni added a commit that referenced this pull request Jun 7, 2024
* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
zalimeni added a commit that referenced this pull request Jun 7, 2024
* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
zalimeni added a commit that referenced this pull request Jun 7, 2024
… release/1.5.x (#4094)

Backport of [NET-9839] fix: add shared version submodule (#4091)

* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
zalimeni added a commit that referenced this pull request Jun 7, 2024
* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
zalimeni added a commit that referenced this pull request Jun 7, 2024
* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
zalimeni added a commit that referenced this pull request Jun 7, 2024
… release/1.5.0 (#4095)

Backport of [NET-9839] fix: add shared version submodule (#4091)

* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
zalimeni added a commit that referenced this pull request Jun 7, 2024
… release/1.4.x (#4096)

Backport of [NET-9839] fix: add shared version submodule (#4091)

* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
zalimeni added a commit that referenced this pull request Jun 7, 2024
… release/1.3.x (#4097)

Backport of [NET-9839] fix: add shared version submodule (#4091)

* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
zalimeni added a commit that referenced this pull request Jun 7, 2024
… release/1.1.x (#4098)

Backport of [NET-9839] fix: add shared version submodule (#4091)

* fix: add shared version submodule

Introduce a new submodule for sharing versioning code across the
binary-producing submodules in `consul-k8s` repo.

This is both an improvement in terms of consolidation, as well as a fix
for incorrect `control-plane` submodule pinning in the
`control-plane/cni` submodule that led to incorrect dev versions being
used in 1.4.2 and 1.4.3 (the first releases that included a valid
version of `control-plane/cni`. See #4054 comments for more context.

* build: fix non-dev prepare script chart version setting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-backport signals that a PR will not contain a backport label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants