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

Assorted build system and plugin metadata improvements #54

Merged
merged 4 commits into from
Sep 18, 2018

Conversation

michaelklishin
Copy link
Contributor

This started as a small PR that only added rabbit as a dependency but ended up being a bunch of build system improvements that I had to make to build the plugin from source against the tip of RabbitMQ 3.7.x.

While at it, correct version number to follow the convention
all Tier 1 plugins use (X.Y.Z not vX.Y.Z).
 * Use COPY and wildcards instead of ADD in Dockerfile as it's a recommended
   best practice
 * Use 3.7.7 in Docker images
 * Move this plugin's targets to the bottom of the file
@deadtrickster
Copy link
Owner

Thanks!

When I do fresh builds (without deps populated) I always see this:

 DEP    rabbitmq_management
ERROR: Repository not found.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

no matter master or tag. Despite this build goes ok. Would be cool to get rid of this while we are on it. Any idea? Or this PR solves it already?

@michaelklishin
Copy link
Contributor Author

@deadtrickster that's because it assumes that rabbitmq-common must be cloned from your account. It then falls back to the RabbitMQ org. You can run it with V=2 and see what git commands are used.

This PR doesn't solve this as the logic comes from rabbitmq-components.mk and allows a network-local git mirror to be used if needed (e.g. we use mirrors on CI for much faster clones). @dumbbell is there are way to force rabbitmq org clones by default for this specific project?

@dumbbell
Copy link

We can modify rabbitmq-components.mk to throw the output of git-clone(1) to /dev/null.

@deadtrickster
Copy link
Owner

imo throwing everything to /dev/null is too much. it could just discard output of first network-local clone. or store it somewhere and present iff all cloning attempts failed.

@deadtrickster deadtrickster merged commit d3f083f into deadtrickster:master Sep 18, 2018
@deadtrickster
Copy link
Owner

after merging I have this error:

    prometheus_rabbitmq_exporter:
        Plugin doesn't support current server version. Actual broker version: "3.7.0+rc.2.138.g8f71523", supported by the plugin: ["3.7.2-3.7.x"]

I'm on master and git log for deps/rabbit:

commit 8f71523f001aa484fa514724dee15ed9ace87adc (HEAD -> master, origin/master, origin/HEAD)
Merge: 3d08081d8 7a0c8b6f9
Author: Michael Klishin <michael@novemberain.com>
Date:   Mon Sep 17 10:41:36 2018 +0200

    Merge pull request #1700 from rabbitmq/rabbitmq-server-1699
    
    Make pg_local:member_died/2 more resilient

commit 7a0c8b6f95067446c51b9e1d2eba21d655e41503
Author: Michael Klishin <michael@clojurewerkz.org>
Date:   Mon Sep 17 10:31:10 2018 +0200

    Clarify

@michaelklishin
Copy link
Contributor Author

@deadtrickster I'll ask around how we solve this for other plugins. Worst case we can drop the requirement. IIRC server version of 0.0.0 should be accepted by any requirements string.

@michaelklishin
Copy link
Contributor Author

It turns out in the 3.7.x series we switched to using Git tag objects (instead of lightweight tags that are references), so git-describe in master won't list the most recent version. I'll submit a PR that removes broker_version_requirements until we resolve this.

@deadtrickster
Copy link
Owner

I commented it already :-)

michaelklishin added a commit to michaelklishin/prometheus_rabbitmq_exporter that referenced this pull request Sep 18, 2018
DXist pushed a commit to DXist/prometheus_rabbitmq_exporter that referenced this pull request Dec 28, 2018
Assorted build system and plugin metadata improvements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants