Kubetest2 Version Addition #111

supriya-premkumar · 2021-03-12T08:50:13Z

Uses ldflags to capture HEAD rev in short form.
Prints built commit version by default
Adds --version support
Fixes partially Add a version command #108

k8s-ci-robot · 2021-03-12T08:50:21Z

Welcome @supriya-premkumar!

It looks like this is your first PR to kubernetes-sigs/kubetest2 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubetest2 has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2021-03-12T08:50:21Z

Hi @supriya-premkumar. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

supriya-premkumar · 2021-03-12T08:51:15Z

/assign @amwat @BenTheElder @dims

spiffxp · 2021-03-14T20:07:31Z

/ok-to-test

spiffxp

/lgtm
Code looks sound

/hold
Deferring to @amwat for approval

Looks like this gets version in the log. What needs to happen to get it in metadata.json and picked up by kettle?

Do we need multiple versions for different kubetest2 binaries? Deployer at one version tester at another sort of thing?

amwat

/approve

Thank you for working on this!

lgtm after 2 suggestions

Also, we want to do this for all the deployer and tester binaries as well, but fine if it's not in this PR (in which case let's modify the PR description so that it won't close the issue.)

pkg/app/shim/shim.go

amwat · 2021-03-15T21:58:55Z

Makefile

@@ -21,6 +21,8 @@
 REPO_ROOT:=$(shell pwd)
 export REPO_ROOT
 OUT_DIR=$(REPO_ROOT)/bin
+# record the source commit in the binary, overridable
+COMMIT?=$(shell git rev-parse --short HEAD 2>/dev/null)


Suggested change

COMMIT?=$(shell git rev-parse --short HEAD 2>/dev/null)

COMMIT?=$(shell git describe --tags --always --dirty 2>/dev/null)

@amwat Just want to make sure that you won't see a -dirty suffix if there are any untracked files.
-dirty works only on tracked diffs. Is this the expectation?

if we can add that as well, that would be great :) but not a huge blocker.

maybe with git status --porcelain and manually add a -dirty suffix if we find untracked files?

amwat · 2021-03-15T23:34:11Z

Looks like this gets version in the log. What needs to happen to get it in metadata.json and picked up by kettle?

We don't produce a metadata.json file yet. Should be fairly straightforward to generate one with the version populated under RunDir().

Likely a good new functionality addition under pkg/app/metadata.

Fine if it's a follow up and left out from this PR.

supriya-premkumar · 2021-03-16T07:14:58Z

Also, we want to do this for all the deployer and tester binaries as well, but fine if it's not in this PR

I didn't know that every deployer gets built as a separate binary. Let me fix that.

supriya-premkumar · 2021-03-16T16:07:51Z

/retest

amwat

Handling deployers will be tricky since we want to support out of tree deployers/testers as well (but we'll be adding the functionality only to the in-tree ones).

I'm fine leaving out the deployer versions from this PR, can be done as a follow up.

What we'd want to achieve that is:

For deployers:

add BUILD_FLAGS specifically in the install-deployer-% make rule
populate a var at build time like we already are doing.
would be good to follow the same convention, say var GitTag string under respective main.go
plumb this through by adding a new callback (for backwards compatibility) func MainWithVersion(deployerName string, deployerVersion string, newDeployer types.NewDeployer).
modify Run to accept version (will be passed as empty from Main) func Run(deployerName string, deployerVersion string, newDeployer types.NewDeployer) error -> runE(cmd, args, deployerName, deployerVersion, newDeployer)

For testers:

add BUILD_FLAGS specifically in the install-tester-% make rule
populate a var at build time like we already are doing.
would be good to follow the same convention, say var GitTag string under respective main.go
invoke the tester with --version similar to how we handle --help for testers (but don't fail on errors) https://github.com/kubernetes-sigs/kubetest2/blob/master/pkg/app/cmd.go#L95-L105

pkg/app/cmd.go

amwat · 2021-03-16T19:15:16Z

Makefile

@@ -47,7 +49,7 @@ SPACE:=$(subst ,, )
 SHELL:=env PATH=$(subst $(SPACE),\$(SPACE),$(PATH)) $(SHELL)
 # ==============================================================================
 # flags for reproducible go builds
-BUILD_FLAGS?=-trimpath -ldflags="-buildid="
+BUILD_FLAGS?=-trimpath -ldflags="-buildid= -X=sigs.k8s.io/kubetest2/pkg/app.GitTag=$(COMMIT)"


unfortunately, handling the deployers won't be so straightforward :)
this will just always get the kubetest2 shim version. but we can have deployers built at different commits.

supriya-premkumar · 2021-03-22T18:08:07Z

I was held up last week. I will work on it this week, thank you.

- Uses ldflags to capture HEAD rev in short form. - Prints built commit version by default - Adds --version support - Fixes 108

amwat

getting the version for the main binary is a good start

/lgtm
/approve
/hold

modifying the PR description so it doesn't close the issue

amwat · 2021-04-02T00:27:33Z

pkg/app/cmd.go

@@ -67,6 +67,8 @@ func runE(
 	opts.bindFlags(kubetest2Flags)
 	artifacts.MustBindFlags(kubetest2Flags)

+	cmd.Printf("Running deployer %s version: %s\n", deployerName, shim.GitTag)


we can add this after deployer versions are added

amwat · 2021-04-02T00:27:41Z

pkg/app/cmd.go

@@ -91,6 +93,7 @@ func runE(
 		if err != nil {
 			return fmt.Errorf("unable to find tester %v: %v", opts.test, err)
 		}
+		cmd.Printf("Running tester %s version: %s\n", opts.test, shim.GitTag)


we can add this after testers versions are added

k8s-ci-robot · 2021-04-02T00:28:38Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amwat, supriya-premkumar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [amwat]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

BenTheElder · 2021-04-02T01:24:58Z

we might be able to get clever with https://golang.org/pkg/runtime/debug/#BuildInfo for out of tree stuff importing the repo

MushuEE · 2021-04-05T20:47:32Z

pkg/app/shim/shim.go

 	// there should be at least one argument (the deployer) unless the user
 	// is asking for help on the shim itself
 	if len(args) < 1 {
 		return cmd.Help()
 	}

-	// gracefully handle -h or --help if it is the only argument
+	// gracefully handle help or version command if it is the only argument


Unrelated to this feature, but if other arguments are set AND --help is included, it will try to deploy?

no

kubetest2/pkg/app/cmd.go

Lines 139 to 143 in 96ab8c9

// print usage and return if no args are provided, or help is explicitly requested

if len(args) == 0 || opts.HelpRequested() {

cmd.Print(usage.String())

return nil

}

BenTheElder · 2021-06-25T06:35:14Z

@amwat perhaps unhold and iterate?

amwat · 2021-06-25T08:54:59Z

yeah, I think we can iterate on this

/unhold

k8s-ci-robot requested review from MushuEE and spiffxp March 12, 2021 08:50

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 12, 2021

k8s-ci-robot assigned amwat, BenTheElder and dims Mar 12, 2021

supriya-premkumar force-pushed the kubetest2-version-addition branch from aec704c to ac10ca5 Compare March 12, 2021 08:54

spiffxp reviewed Mar 14, 2021

View reviewed changes

k8s-ci-robot assigned spiffxp Mar 14, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 14, 2021

amwat reviewed Mar 15, 2021

View reviewed changes

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 15, 2021

supriya-premkumar force-pushed the kubetest2-version-addition branch from ac10ca5 to f5249f6 Compare March 16, 2021 06:56

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 16, 2021

supriya-premkumar force-pushed the kubetest2-version-addition branch from f5249f6 to 5d70c64 Compare March 16, 2021 09:04

supriya-premkumar requested review from amwat and spiffxp March 16, 2021 09:04

supriya-premkumar force-pushed the kubetest2-version-addition branch from 5d70c64 to 4c94ca1 Compare March 16, 2021 16:19

supriya-premkumar force-pushed the kubetest2-version-addition branch from 4c94ca1 to 4526caf Compare March 16, 2021 16:31

amwat reviewed Mar 16, 2021

View reviewed changes

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 25, 2021

supriya-premkumar force-pushed the kubetest2-version-addition branch from 4526caf to 5135f73 Compare March 31, 2021 03:11

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 31, 2021

supriya-premkumar force-pushed the kubetest2-version-addition branch from 5135f73 to f369bc1 Compare March 31, 2021 05:39

Kubetest2 Version Addition

f369bc1

- Uses ldflags to capture HEAD rev in short form. - Prints built commit version by default - Adds --version support - Fixes 108

amwat reviewed Apr 2, 2021

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 2, 2021

MushuEE reviewed Apr 5, 2021

View reviewed changes

supriya-premkumar mentioned this pull request Apr 28, 2021

REQUEST: New membership for supriya-premkumar kubernetes/org#2662

Closed

supriya-premkumar mentioned this pull request Jun 15, 2021

Make it clear which version of kubetest or kubetest2 is being used kubernetes/test-infra#20886

Closed

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jun 25, 2021

k8s-ci-robot merged commit cb5f2dc into kubernetes-sigs:master Jun 25, 2021

BenTheElder mentioned this pull request Jun 25, 2021

Add a version command #108

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Kubetest2 Version Addition #111

Kubetest2 Version Addition #111

supriya-premkumar commented Mar 12, 2021 •

edited by amwat

Loading

k8s-ci-robot commented Mar 12, 2021

k8s-ci-robot commented Mar 12, 2021

supriya-premkumar commented Mar 12, 2021

spiffxp commented Mar 14, 2021

spiffxp left a comment

amwat left a comment

amwat Mar 15, 2021

supriya-premkumar Mar 16, 2021

amwat Mar 16, 2021

amwat commented Mar 15, 2021

supriya-premkumar commented Mar 16, 2021 •

edited

Loading

supriya-premkumar commented Mar 16, 2021

amwat left a comment

amwat Mar 16, 2021

supriya-premkumar commented Mar 22, 2021

amwat left a comment

amwat Apr 2, 2021

amwat Apr 2, 2021

k8s-ci-robot commented Apr 2, 2021

BenTheElder commented Apr 2, 2021

MushuEE Apr 5, 2021

amwat Apr 5, 2021

BenTheElder commented Jun 25, 2021

amwat commented Jun 25, 2021

	COMMIT?=$(shell git rev-parse --short HEAD 2>/dev/null)
	COMMIT?=$(shell git describe --tags --always --dirty 2>/dev/null)

	// print usage and return if no args are provided, or help is explicitly requested
	if len(args) == 0 \|\| opts.HelpRequested() {
	cmd.Print(usage.String())
	return nil
	}

Kubetest2 Version Addition #111

Kubetest2 Version Addition #111

Conversation

supriya-premkumar commented Mar 12, 2021 • edited by amwat Loading

k8s-ci-robot commented Mar 12, 2021

k8s-ci-robot commented Mar 12, 2021

supriya-premkumar commented Mar 12, 2021

spiffxp commented Mar 14, 2021

spiffxp left a comment

Choose a reason for hiding this comment

amwat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amwat commented Mar 15, 2021

supriya-premkumar commented Mar 16, 2021 • edited Loading

supriya-premkumar commented Mar 16, 2021

amwat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

supriya-premkumar commented Mar 22, 2021

amwat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 2, 2021

BenTheElder commented Apr 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenTheElder commented Jun 25, 2021

amwat commented Jun 25, 2021

supriya-premkumar commented Mar 12, 2021 •

edited by amwat

Loading

supriya-premkumar commented Mar 16, 2021 •

edited

Loading