Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Log binary versions #216

Merged
merged 7 commits into from
May 17, 2019
Merged

Conversation

CecileRobertMichon
Copy link
Contributor

@CecileRobertMichon CecileRobertMichon commented May 16, 2019

Acceptance criteria: As a user, I would like to see binary versions of NMI and MIC in the logs.

Fix a typo in the Makefile that caused the ARG for NMI_VERSION to be always empty. Add handling for MIC_VERSION.

These versions will show in the logs next time we build an image using NMI_VERSION and MIC_VERSION args.

Fixes #207

@CecileRobertMichon
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 216 in repo Azure/aad-pod-identity

cmd/nmi/main.go Outdated Show resolved Hide resolved
var MICVersion string

// NMIVersion is the version of the NMI component
var NMIVersion string

// PrintVersionAndExit prints the version and exits
func PrintVersionAndExit() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be used in case of -v command line option for both mmi and mic. Would be a good way to find out the comparable nmi/mic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do we really want to exit the process after printing the version?

Copy link
Contributor

@kkmsft kkmsft May 16, 2019

Choose a reason for hiding this comment

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

I was thinking if so if some one runs

nmi -v
or
mic -v

this is the only which needs to be called. In that case the exit can be left there as is - thoughts ?

Also wondering why the fmt was used here originally, does this assume that blog is not initialized or some other rule to be followed in version.go - any ideas ?

flag.Parse()
if versionInfo {
version.PrintVersionAndExit()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this what you meant @kkmsft ?

@kkmsft
Copy link
Contributor

kkmsft commented May 17, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

cmd/mic/main.go Outdated Show resolved Hide resolved
cmd/nmi/main.go Outdated Show resolved Hide resolved
@CecileRobertMichon
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 216 in repo Azure/aad-pod-identity

@kkmsft
Copy link
Contributor

kkmsft commented May 17, 2019

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kkmsft kkmsft merged commit 9e675db into Azure:master May 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

include binary versions in logs
2 participants