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

openstack-cloud-controller-manager can not show version #907

Closed
1 task done
RainbowMango opened this issue Jan 16, 2020 · 8 comments · Fixed by #950
Closed
1 task done

openstack-cloud-controller-manager can not show version #907

RainbowMango opened this issue Jan 16, 2020 · 8 comments · Fixed by #950
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug.

Comments

@RainbowMango
Copy link
Member

Is this a BUG REPORT or FEATURE REQUEST?:
/kind bug

The binaries affected:

  • openstack-cloud-controller-manager

What happened:
Invalid version information from openstack-cloud-controller-manager --version.

[root@ecs-d8b6 cloud-provider-openstack]# make openstack-cloud-controller-manager
CGO_ENABLED=0 GOOS=linux go build \
-ldflags "-w -s -X 'k8s.io/cloud-provider-openstack/pkg/version.Version=v1.17.0-34-asfasd'" \
-o openstack-cloud-controller-manager \
cmd/openstack-cloud-controller-manager/main.go
[root@ecs-d8b6 cloud-provider-openstack]# ./openstack-cloud-controller-manager --version
Kubernetes v0.0.0-master+$Format:%h$

What you expected to happen:

How to reproduce it:

Anything else we need to know?:

Environment:

  • openstack-cloud-controller-manager version: 1.17.0
  • OpenStack version: N/A
  • Others:
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jan 16, 2020
@RainbowMango
Copy link
Member Author

cc @dims
Is this a problem and any idea about it?

@mikejoh
Copy link
Contributor

mikejoh commented Jan 16, 2020

Interesting, could it be due to us calling this function:

That function was imported from k8s.io/kubernetes/pkg/version/verflag (pre k8s 1.17.0 at least). So it looks like that our version.Version never gets outputted but instead something related to kubernetes itself? The -X flag is set correctly at compile time and the version.Version prints the version set through the Makefile.

Bascially we should probably change the way we print the version, maybe use constructs within cobra?

@RainbowMango
Copy link
Member Author

So it looks like that our version.Version never gets outputted but instead something related to kubernetes itself?

Yes. The version information comes from Kubernetes. And the information is wrong because of we are build outside of Kubernetes, so no chance to inject version infor to gitVersion .
https://github.com/kubernetes/kubernetes/blob/718714a9359e628d43aa1005301427983b760b59/staging/src/k8s.io/component-base/version/base.go#L58.

@jichenjc
Copy link
Contributor

seems the others like cinder-csi-plugin doesn't have this
should we consider use same pattern for all exec file we compiled ?

@mikejoh
Copy link
Contributor

mikejoh commented Jan 20, 2020

@jichenjc Yes you're correct, the cinder-csi-plugin doesn't expose a --version flag. Haven't looked through the rest of them though. I think it's a good idea to at least have a way of printing the version and if possible do it in a unified way across all binaries compiled within the cloud-provider-openstack project. I'll try look into it this week, or have you started perhaps? Let me know if you need help anyways!

@jichenjc
Copy link
Contributor

I can help the review since you already have the plan :) thanks a lot

@ramineni
Copy link
Contributor

@mikejoh Im assigning this to you , if you don't have plans to fix , pls unassign yourself.
/assign @mikejoh

@mikejoh
Copy link
Contributor

mikejoh commented Jan 23, 2020

@ramineni Thanks! 👍🏻 I will try to fix this one as soon as i’m back from vacation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants