move chef -v to golang #2062

i5pranay93 · 2021-06-10T15:07:48Z

Adding the option for the chef -v through go wrapper

Description

Provides the option to pass -v/--version through chef-main-wrapper in golang, with a function version() where all the logic from chef-cli with ruby code is pulled and switched with golang

Related Issue

fixes #656
Related PR which will replace this in future #1465

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

I have read the CONTRIBUTING document.
I have run the pre-merge tests locally and they pass.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.
All commits have been signed-off for the Developer Certificate of Origin.

netlify · 2021-06-10T15:07:55Z

👷 Deploy Preview for chef-workstation processing.

🔨 Explore the source changes: a80f130

🔍 Inspect the deploy log: https://app.netlify.com/sites/chef-workstation/deploys/611cebafbe381100087a6cb4

tas50 · 2021-06-10T18:12:42Z

Before:

After:

marcparadise

I think it would make sense to move the implementation here as well. Moving the flag itself but still invoking chef-cli won't get the improvements that are requested in the original issue.

An implementation written for this PR won't be invalidated by #1465, though it may need to be updated a bit

marcparadise · 2021-06-21T16:48:19Z

components/main-chef-wrapper/lib/version.go

+	"path"
+	"io/ioutil"
+	"encoding/json"
+	"github.com/mnogu/go-dig"


This looks helpful, but I'm concerned because it looks like it was created over the course of 1-2 days and has not received any attention since then.

I think I saw some talk around defining a nested struct to unmarshal into. While we can't do that for the gem version manifest, is it possible to do it for the workstation versoin manifest? Or is the structure of that file problematic, eg there's no array of definitions under software, but rather a bunch of named json objects there?

i think we can unmarshall gem manifest since it seems structured,
eg-

"wmi-lite": [ "1.0.5" ], "xmlrpc": [ "0.3.0" ],

but the structure of the file version manifest under software is a bunch of JSON object so two different structures were there in both file.

So I made it into interface which made it easier for me.

This looks helpful, but I'm concerned because it looks like it was created over the course of 1-2 days and has not received any attention since then.

are we talking about this "github.com/mnogu/go-dig"??, I think code was pretty straight. i could have written same for it saved time and space for me.
edited -Hm, Seems its failing in buildkite. i will write it manually

marcparadise · 2021-06-21T16:53:04Z

components/main-chef-wrapper/lib/version.go

+	if omnibusInstall() == true {
+		showVersionViaVersionManifest()
+	} else {
+		// todo "5.1,0 is coming from #{ChefCLI::VERSION} in ruby, need to figure out how to do this "


I think we can eliminate the branch that does not come from an omnibus install, or consider it an error. This wrapper is only supported in an omnibus install.

Okay, i will raise error.

i5pranay93

pushed changes in new commits

components/main-chef-wrapper/lib/dig.go

marcparadise · 2021-06-23T15:07:40Z

This looks good, but let's add some level of testing for Dig as well.

tas50 · 2021-06-29T07:45:24Z

components/main-chef-wrapper/lib/version.go

+func omnibusRoot()string  {
+	omnibusroot, err := filepath.Abs(path.Join(ExpectedOmnibusRoot()))
+	if err != nil {
+		fmt.Fprintln(os.Stderr, "ERROR:", "Can not find omnibus installation directory for Chef Workstation")


Should this error also use the dist file?

as in Chef Workstation part from "Can not find omnibus installation directory for Chef Workstation" can be used from dist ile ??

Updated it with fmt.Fprintln(os.Stderr, "ERROR:", "Can not find omnibus installation directory for", dist.WorkstationProduct)

tas50 · 2021-06-29T07:45:51Z

This should get rebased on master

tas50 · 2021-06-29T07:52:45Z

adhoc to confirm functionality: https://buildkite.com/chef/chef-chef-workstation-master-omnibus-adhoc/builds/501#02ff98c2-cc72-4bda-a52b-0a66b0a63862

marcparadise · 2021-06-29T13:12:25Z

components/main-chef-wrapper/lib/version.go

+}
+
+func ExpectedOmnibusRoot()string {
+	groot := os.Getenv("GEM_ROOT")


I don't think we can assume this will be present in the user's environment because we're no longer loading this in the context of the ruby environment.

Instead, we should look relative to the chef executable directory.

marcparadise · 2021-07-02T20:21:06Z

components/main-chef-wrapper/lib/version.go

+	if omnibusInstall() == true {
+		showVersionViaVersionManifest()
+	} else {
+		fmt.Fprintln(os.Stderr, "ERROR:", "Can not find omnibus installation directory for", dist.WorkstationProduct)


This message won't necessarily mean much to the person using it, who doesn't really have awareness of 'omnibus'. Instead, maybe "$PRODUCT has not been installed via the platform-specific package provided by $DISTRIBUTOR-NAME. Version information is not available."?

According to my understanding,
$PRODUCT - would be a dynamic value containing e.g (Chef-workstation)
what would be $DISTRIBUTOR-NAME?? or may be I am understanding it wrong

marcparadise

Comments are inline. Please delete your local branch before pulling this down to avoid conflicts/merge breakage.

tas50 · 2021-07-02T23:26:41Z

I ran an adhoc of this and it's failing to pull the version on my mac:

❰tsmith❙~❱✔≻ chef -v
open /gem-version-manifest.json: no such file or directory
open /version-manifest.json: no such file or directory
ERROR: Can not find omnibus installation directory for Chef Workstation

components/main-chef-wrapper/lib/version.go

marcparadise · 2021-07-14T16:24:50Z

This is looking good - we're almost there.

Two things remaining - let's check the caess where we have errors that we can't reasonably recover from, and exit at those locations or return the errors to the caller(s) so that they can exit gracefully.

Separately, please run go fmt to format the source files.

i5pranay93

Reviwed

i5pranay93 · 2021-07-08T12:37:05Z

components/main-chef-wrapper/lib/version.go

+	if omnibusInstall() == true {
+		showVersionViaVersionManifest()
+	} else {
+		fmt.Fprintln(os.Stderr, "ERROR:", "Can not find omnibus installation directory for", dist.WorkstationProduct)


According to my understanding,
$PRODUCT - would be a dynamic value containing e.g (Chef-workstation)
what would be $DISTRIBUTOR-NAME?? or may be I am understanding it wrong

Provides the option to pass -v/--version through chef-main-wrapper in golang, with a function version() where all the logic from chef-cli with ruby code is pulled and switched with golang This removes the overhead of starting the ruby VM to report version , most of which is loaded from files distributed in the chef-workstation build. Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

…ibutor name in constat Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

i5pranay93 requested review from a team as code owners June 10, 2021 15:07

marcparadise requested changes Jun 10, 2021

View reviewed changes

marcparadise reviewed Jun 21, 2021

View reviewed changes

i5pranay93 commented Jun 22, 2021

View reviewed changes

i5pranay93 requested a review from marcparadise June 22, 2021 19:43

marcparadise reviewed Jun 23, 2021

View reviewed changes

components/main-chef-wrapper/lib/dig.go Show resolved Hide resolved

tas50 reviewed Jun 29, 2021

View reviewed changes

marcparadise reviewed Jun 29, 2021

View reviewed changes

i5pranay93 requested a review from a team as a code owner June 30, 2021 12:15

marcparadise force-pushed the pranay/move_chef_v_to_go_656 branch from 74c45b4 to 4ff6ab3 Compare July 2, 2021 20:15

marcparadise reviewed Jul 2, 2021

View reviewed changes

marcparadise requested changes Jul 2, 2021

View reviewed changes

kagarmoe removed the request for review from a team July 8, 2021 18:50

marcparadise reviewed Jul 12, 2021

View reviewed changes

components/main-chef-wrapper/lib/version.go Outdated Show resolved Hide resolved

marcparadise reviewed Jul 14, 2021

View reviewed changes

components/main-chef-wrapper/lib/version.go Outdated Show resolved Hide resolved

i5pranay93 commented Jul 28, 2021

View reviewed changes

i5pranay93 force-pushed the pranay/move_chef_v_to_go_656 branch from cf993f1 to 8a3ef2b Compare August 17, 2021 14:51

i5pranay93 added 4 commits August 18, 2021 16:44

Resolving path detection problem for adhoc build

2fd624f

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

leveling one level up path directory and passing on Product and distr…

37e8c11

…ibutor name in constat Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

return exit code for non recovering error

8fa1741

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

i5pranay93 added 8 commits August 18, 2021 16:44

running go fmt for formatting

b0d723a

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

Adding registry code to windows

58c9aec

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

window build for regustry

0c293d7

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

getting root path from registry in windows

a708023

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

replace all the occurence of // with / in root path in windows

8c6cde3

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

seperating windows and unix platform to make seperate build

abc1db8

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

adding platform based files

fa2df37

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

platform specific comment for files

a80f130

Signed-off-by: Pranay Singh <i5singh.pranay@gmail.com>

i5pranay93 force-pushed the pranay/move_chef_v_to_go_656 branch from 8a3ef2b to a80f130 Compare August 18, 2021 11:14

tas50 merged commit 971d42e into main Aug 25, 2021

tas50 deleted the pranay/move_chef_v_to_go_656 branch August 25, 2021 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

move chef -v to golang #2062

move chef -v to golang #2062

i5pranay93 commented Jun 10, 2021 •

edited

Loading

netlify bot commented Jun 10, 2021 •

edited

Loading

tas50 commented Jun 10, 2021

marcparadise left a comment

marcparadise Jun 21, 2021

marcparadise Jun 21, 2021

i5pranay93 Jun 21, 2021

i5pranay93 Jun 21, 2021 •

edited

Loading

marcparadise Jun 21, 2021 •

edited

Loading

i5pranay93 Jun 21, 2021

i5pranay93 left a comment

marcparadise commented Jun 23, 2021

tas50 Jun 29, 2021

i5pranay93 Jun 30, 2021

i5pranay93 Jun 30, 2021

tas50 commented Jun 29, 2021

tas50 commented Jun 29, 2021

marcparadise Jun 29, 2021

marcparadise Jul 2, 2021

i5pranay93 Jul 8, 2021

marcparadise left a comment

tas50 commented Jul 2, 2021 •

edited

Loading

marcparadise commented Jul 14, 2021

i5pranay93 left a comment

i5pranay93 Jul 8, 2021

move chef -v to golang #2062

move chef -v to golang #2062

Conversation

i5pranay93 commented Jun 10, 2021 • edited Loading

Description

Related Issue

Types of changes

Checklist:

netlify bot commented Jun 10, 2021 • edited Loading

tas50 commented Jun 10, 2021

marcparadise 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

i5pranay93 Jun 21, 2021 • edited Loading

Choose a reason for hiding this comment

marcparadise Jun 21, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i5pranay93 left a comment

Choose a reason for hiding this comment

marcparadise commented Jun 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tas50 commented Jun 29, 2021

tas50 commented Jun 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcparadise left a comment

Choose a reason for hiding this comment

tas50 commented Jul 2, 2021 • edited Loading

marcparadise commented Jul 14, 2021

i5pranay93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i5pranay93 commented Jun 10, 2021 •

edited

Loading

netlify bot commented Jun 10, 2021 •

edited

Loading

i5pranay93 Jun 21, 2021 •

edited

Loading

marcparadise Jun 21, 2021 •

edited

Loading

tas50 commented Jul 2, 2021 •

edited

Loading