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

Incorporate Adapter Version # at buildtime #25

Closed
leecalcote opened this issue Dec 4, 2020 · 24 comments
Closed

Incorporate Adapter Version # at buildtime #25

leecalcote opened this issue Dec 4, 2020 · 24 comments
Assignees
Labels
area/ci Continuous integration | Build and release issue/willfix This issue will be worked on kind/enhancement Improvement in current feature

Comments

@leecalcote
Copy link
Member

Current Behavior
Meshery adapters have their version # hardcoded. See https://github.com/layer5io/meshery-adapter-library/blob/master/common/defaults.go#L34

Desired Behavior
An adapter's version number should be retrieved at buildtime from the CI workflow.


Resources

Alternatives / Additional Context

@leecalcote leecalcote added area/ci Continuous integration | Build and release kind/enhancement Improvement in current feature labels Dec 4, 2020
@mgfeller
Copy link
Contributor

mgfeller commented Jan 4, 2021

I think adding a new proto functions is a good solution. Maybe we could add a more general function, like MeshInfo, with a map, so that we can send over additional information without having to change the proto definition. Also, we'd like to send over both GIT_VERSION and GIT_COMMITSHA.

The function call can be handled in meshery-adapter-library , like GetName (adapter/spec.go:30), which again is called by the proto function MeshName (api/grpc/handlers.go:36). There is actually already a function GetVersion in adapter/spec.go, but appears unused. Maybe it needs modification.

In the adapters, the build-and-release.yaml and Dockerfile have to be modified, and in main.go (possibly), the version in the config has to be set.
Similar has been done in meshery , build-and-release.yaml :
--build-arg GIT_COMMITSHA=${GITHUB_SHA::7} --build-arg GIT_VERSION=${GITHUB_REF/refs\/tags\//}
Dockerfile :
-X main.version=$GIT_VERSION -X main.commitsha=$GIT_COMMITSHA

Any more general functionality should go into meshkit, but right now I can't see any.

@mgfeller
Copy link
Contributor

mgfeller commented Jan 4, 2021

Of course, it should be ServerInfo rather than MeshInfo for this issue, my mistake. Also, the GetVersion returns the mesh version.
Personally I'd prefer AdapterInfo, because it describes the function, and not the implementation, as ServerInfo does. We usually use the term 'adapter' rather than 'server' as well.

@leecalcote
Copy link
Member Author

@mgfeller, you're right that there is an argument for inclusion of this component-version-reporting functionality to land in MeshKit. For example, consider this mockup:

Settings

If you have motioned to move this enhancement to meshkit, then I think I have just seconded that motion. :)

// @kumarabd @Utkarsh-pro @uds5501

@mgfeller
Copy link
Contributor

mgfeller commented Jan 6, 2021

From Slack:

image

@mgfeller
Copy link
Contributor

mgfeller commented Jan 6, 2021

If you have motioned to move this enhancement to meshkit, then I think I have just seconded that motion. :)

Hmm... I might have mentioned something related to that before (but can't find the conversation).

The individual components will expose this information. For adapters, this means implementing it (also) as a function in the gRPC service, in meshery-adapter-library (leveraging the existing injection mechanisms to expose info from contained in the config).

How are other components exposing information about themselves?

Client functionality could of course be implemented in meshkit.

A ComponentInfo message could be defined centrally in meshkit and used in other components (name, type, status, health, version, commit_sha, id, description, ...).

Could meshops.proto be moved from meshery to meshkit?

// @leecalcote @kumarabd @uds5501 @Utkarsh-pro

@leecalcote
Copy link
Member Author

@mgfeller, good call. @uds5501 the /info endpoint in meshkit makes component version available. See https://github.com/layer5io/meshkit/tree/master/protobuf.

@mgfeller
Copy link
Contributor

mgfeller commented Jan 7, 2021

@leecalcote @uds5501 yes, I thought something along those lines, but more fields, or a map, or a composed message. This is controller info, though, not component.
I envisaged something like /protobuf/component/info.proto with name, id, status, health ++ that would be imported by e.g. controller and adapter service (i.e. into https://github.com/layer5io/meshery/blob/master/meshes/meshops.proto), used in service responses or events. every service could e.g. expose a function/endpoint/event ComponentInfo.

@stale
Copy link

stale bot commented Feb 21, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the issue/stale Issue has not had any activity for an extended period of time label Feb 21, 2021
@mgfeller mgfeller added issue/willfix This issue will be worked on and removed issue/stale Issue has not had any activity for an extended period of time labels Feb 21, 2021
@leecalcote
Copy link
Member Author

Sounds good to me, @mgfeller

@leecalcote
Copy link
Member Author

YES! 😄
Screen Shot 2021-02-21 at 8 19 03 PM

@mgfeller
Copy link
Contributor

mgfeller commented Feb 23, 2021

Issue to expand meshops.proto meshery/meshery#2449, needs to be implemented first.

@mgfeller mgfeller self-assigned this Feb 23, 2021
@mgfeller
Copy link
Contributor

Looks like it would be preferable to merge #34 first.

@leecalcote
Copy link
Member Author

Quick check... this component version information is now coming through and is available to Meshery Server, correct? If so, it's time for an issue to expose this in Meshery UI.

Excited about this one. 😄

@mgfeller
Copy link
Contributor

I'll check the status on this one. And create a Meshery UI issue if it doesn't exist and hasn't been implemented.

// @leecalcote

@mgfeller
Copy link
Contributor

mgfeller commented Jun 22, 2021

// @leecalcote


❯ grpcurl --plaintext 127.0.0.1:10002 meshes.MeshService.ComponentInfo
{
  "type": "adapter",
  "name": "CONSUL",
  "version": "none",
  "gitSha": "none"
}

❯ grpcurl --plaintext 127.0.0.1:10008 meshes.MeshService.ComponentInfo
Error invoking method "meshes.MeshService.ComponentInfo": failed to query for service descriptor "meshes.MeshService": server does not support the reflection API

❯ grpcurl --plaintext 127.0.0.1:10000 meshes.MeshService.ComponentInfo
{
  "type": "adapter",
  "name": "ISTIO",
  "version": "v0.5.9",
  "gitSha": "fa47813"
}

❯ grpcurl --plaintext 127.0.0.1:10007 meshes.MeshService.ComponentInfo
{
  "type": "adapter",
  "name": "KUMA",
  "version": "v0.5.5",
  "gitSha": "a116b03"
}

❯ grpcurl --plaintext 127.0.0.1:10001 meshes.MeshService.ComponentInfo
{
  "type": "adapter",
  "name": "LINKERD",
  "version": "v0.5.5",
  "gitSha": "8cf2b32"
}

❯ grpcurl --plaintext 127.0.0.1:10004 meshes.MeshService.ComponentInfo
{
  "type": "adapter",
  "name": "NETWORK_SERVICE_MESH"
}

❯ grpcurl --plaintext 127.0.0.1:10009 meshes.MeshService.ComponentInfo
{
  "type": "adapter",
  "name": "OPEN_SERVICE_MESH",
  "version": "none",
  "gitSha": "none"
}

❯ grpcurl --plaintext 127.0.0.1:10006 meshes.MeshService.ComponentInfo
{
  "type": "adapter",
  "name": "TRAEFIK_MESH",
  "version": "v0.5.6",
  "gitSha": "9029d0b"
}

@leecalcote
Copy link
Member Author

leecalcote commented Jun 22, 2021

Progress. @mgfeller, in a non-obvious way, we've begun to expose some of this in the UI and show to the user on hover of an adapter chip. We need to do more in this regard; need a separate Meshery System Dashboard.

Screen Shot 2021-06-22 at 3 50 40 PM

@ramrodo, heads-up: we might not be including build information in each of the Meshery Adapter build workflows, at least not for OSM, NSM, Consul, CPX. It should be the case that we can propagate workflow updates to include version and tag into these adapter repos with relatively little pain.

@mgfeller
Copy link
Contributor

Looks like the Consul-adapter just needs to be released, the changes have been made:

image

@mgfeller
Copy link
Contributor

For meshery-nsm see meshery/meshery-nsm#124

@mgfeller
Copy link
Contributor

meshery-osm code looks also good, looks like just a release is needed?

image

@mgfeller
Copy link
Contributor

I suggest this general issue can be closed now - and will be if no-one objects within a few days or so.

// @leecalcote

@mgfeller
Copy link
Contributor

Hmm I would have expected that e.g. Consul edge-latest returns version: edge-latest but it doesn't afaics...

@mgfeller
Copy link
Contributor

mgfeller commented Aug 28, 2021

Hmm I would have expected that e.g. Consul edge-latest returns version: edge-latest but it doesn't afaics...

main.version etc needs to be var not const

meshery/meshery-consul#188

@mgfeller
Copy link
Contributor

mgfeller commented Aug 30, 2021

Hmm I would have expected that e.g. Consul edge-latest returns version: edge-latest but it doesn't afaics...

main.version etc needs to be var not const

Same for meshery-osm...

meshery/meshery-osm#111

@mgfeller
Copy link
Contributor

Consul, NSM, OSM return version and gitsha now, tested with edge and using
grpcurl --plaintext 127.0.0.1:<port> meshes.MeshService.ComponentInfo
(CPX needs to be updated to meshery-adapter-library by the looks of it.)

closing this issue now 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci Continuous integration | Build and release issue/willfix This issue will be worked on kind/enhancement Improvement in current feature
Projects
None yet
Development

No branches or pull requests

2 participants