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 upgrade-candidate #716

Merged
merged 1 commit into from
Nov 9, 2021
Merged

Fix upgrade-candidate #716

merged 1 commit into from
Nov 9, 2021

Conversation

kuritka
Copy link
Collaborator

@kuritka kuritka commented Nov 8, 2021

upgrade-candidate - the target for deploying the current version of
k8gb to local k3d clusters tried to deploy a local image that did not exist.
Here is the fix.

Signed-off-by: kuritka [email protected]

ytsarev
ytsarev previously approved these changes Nov 8, 2021
somaritane
somaritane previously approved these changes Nov 8, 2021
Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

It probably duplicates the build step in the pipeline

@kuritka
Copy link
Collaborator Author

kuritka commented Nov 8, 2021

It probably duplicates the build step in the pipeline

going to fix

@k0da
Copy link
Collaborator

k0da commented Nov 8, 2021

Yes.

  • name: Build artifacts
    uses: goreleaser/goreleaser-action@v2
    with:
    version: latest
    args: release --rm-dist --skip-publish --skip-validate --snapshot
    env:
    GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

@kuritka kuritka dismissed stale reviews from somaritane and ytsarev via e624919 November 8, 2021 13:13
@kuritka kuritka force-pushed the fix-upgrade-candidate branch 8 times, most recently from 6cf7fb1 to 14c83ee Compare November 8, 2021 15:59
@kuritka kuritka requested review from ytsarev and somaritane November 8, 2021 16:00
@kuritka kuritka force-pushed the fix-upgrade-candidate branch from 14c83ee to 77d6839 Compare November 8, 2021 16:10
@kuritka
Copy link
Collaborator Author

kuritka commented Nov 9, 2021

All green, can be reviewed again...

Makefile Outdated
.PHONY: upgrade-candidate
upgrade-candidate: ## Upgrade k8gb to the test version on existing clusters
.PHONY: deploy-semver
deploy-semver: ## Upgrade k8gb to the test version on existing clusters
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
deploy-semver: ## Upgrade k8gb to the test version on existing clusters
deploy-test-version: ## Upgrade k8gb to the test version on existing clusters

I might be missing something but deploy-semver is a bit confusing target name here

Copy link
Collaborator Author

@kuritka kuritka Nov 9, 2021

Choose a reason for hiding this comment

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

The main idea for naming is we operate with stable[1], [2] [3] and semver [4] terms. I was trying to keep same semantics.

Maybe can rename both (deploy-stable, deploy-semver)

Copy link
Member

Choose a reason for hiding this comment

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

Agreed, using the word semver in this context is quite misleading. Semantic versioning talks about the versioning scheme, when to raise each component of that x.y.z triplet. So even the stable actually deploys a version (latest released in dockerhub) that conforms the semver.

The makefile also operate with word "candidate", this is much closer to what it is, imho. Or test-version or latest vs latest-released(=stable).

The version/container tag it actually produces looks like x.y.z-gitsha and from what I've seen it's pretty much the same semantics as the x.y.z-SNAPSHOT versions from the Java world

`upgrade-candidate` - the target for deploying the current version of
k8gb to local k3d clusters tried to deploy a local image that did not exist.
Here is the fix.

The go install command behaves almost identically to go build, but instead
of leaving the executable in the current directory, or a directory specified
by the -o flag, it places the executable into the $GOBIN directory.
goreleaser can be found and removed from there any time.

Signed-off-by: kuritka <[email protected]>
@kuritka kuritka force-pushed the fix-upgrade-candidate branch from 77d6839 to 72a049b Compare November 9, 2021 08:48
@kuritka kuritka requested a review from ytsarev November 9, 2021 09:00
@kuritka
Copy link
Collaborator Author

kuritka commented Nov 9, 2021

@ytsarev , @jkremser, naming fixed

@k0da
Copy link
Collaborator

k0da commented Nov 9, 2021

It still duplicates build, GHA builds image already

@kuritka
Copy link
Collaborator Author

kuritka commented Nov 9, 2021

@k0da are you sure? upgrade-testing.yaml runs exactly same code as before , just target renamed from upgrade-candidate to deploy-test-version.

upgrade-candidate now builds locally + still fits with all documentation.

Copy link
Member

@ytsarev ytsarev left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@k0da
Copy link
Collaborator

k0da commented Nov 9, 2021

@k0da are you sure? upgrade-testing.yaml runs exactly same code as before , just target renamed from upgrade-candidate to deploy-test-version.

upgrade-candidate now builds locally + still fits with all documentation.

https://github.com/k8gb-io/k8gb/blob/master/.github/workflows/upgrade-testing.yaml#L29

@kuritka kuritka merged commit 9dac07a into master Nov 9, 2021
@kuritka kuritka deleted the fix-upgrade-candidate branch November 9, 2021 09:53
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.

5 participants