grafana-ansible-collection icon indicating copy to clipboard operation
grafana-ansible-collection copied to clipboard

Update Alloy variables to use the `grafana_alloy_` namespace so they are unique

Open Aethylred opened this issue 1 year ago • 5 comments
trafficstars

This should address #206 and #194

This PR should make sure the Alloy variables are unique and won't clash with other variables (like version) used elsewhere in playbooks.

It also moves the binary and working directory out of /etc where RHEL and SELinux get upset about.

Aethylred avatar May 21 '24 04:05 Aethylred

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar May 21 '24 04:05 CLAassistant

Hey @Aethylred Yeah would prefer to use alloy_ instead of grafana_alloy_

ishanjainn avatar Jun 03 '24 06:06 ishanjainn

Switched to alloy_ as requested, though I suppose the code is ahead of this now.

Apologies about the delay, with the Alloy setup being immature, we switched to using the standard Grafana setup. Will be skipping Grafana Agent & Flow and waiting for Alloy.

Aethylred avatar Jul 12 '24 01:07 Aethylred

https://github.com/grafana/grafana-ansible-collection/pull/228

BOth of the PRs seem to achive the same thing, @Aethylred Lemme know if the other PR loogs good to you aswell and we can use that or the other way around?

ishanjainn avatar Jul 12 '24 13:07 ishanjainn

Looking at #228 and it looks good, I'd consider it a duplicate, but whichever crosses the line first is fine by me

Aethylred avatar Aug 08 '24 03:08 Aethylred

Rebased on main. Not sure how to resolve the workflow errors.

Aethylred avatar Sep 05 '24 03:09 Aethylred

Any updates on this one? Looks like the only thing missing is approval for workflows to run

PabloEForgeFlow avatar Sep 17 '24 10:09 PabloEForgeFlow

Changes LGTM, Ill just test on local by tomorrow and merge release if all good

ishanjainn avatar Sep 17 '24 11:09 ishanjainn

Hello ! Do you have any updates on this MR ? When will it be merge ? I don't know the process here

It looks like @ishanjainn is pending reviewer

doyl54 avatar Sep 30 '24 06:09 doyl54

Looking at those failed checks and I'm unsure if the problem is my MR, should I rebase again?

Aethylred avatar Oct 01 '24 20:10 Aethylred

Dont think so it related to your changes, Chnages LGTM

ishanjainn avatar Oct 01 '24 22:10 ishanjainn

Hello @ishanjainn Thanks for merging. Please review comment: 2388057978, because it broke #212!

It reverts this:

git diff 8dbd124a04d2d68689fa7fa3be91db3b077b7589 install.yml
index b78a953..d39c97b 100644
--- a/roles/alloy/tasks/install.yml
+++ b/roles/alloy/tasks/install.yml
@@ -6,37 +6,55 @@
...
 - name: Create alloy user
   ansible.builtin.user:
-    name: "{{ service_user }}"
-    groups: "{{ [ service_group ] + alloy_user_groups }}"
+    name: "{{ alloy_service_user }}"
+    group: "{{ alloy_service_group }}"
     system: true
     create_home: false  # Appropriate for a system user, usually doesn't need a home directory
   become: true
...

The {{ [ service_group ] + alloy_user_groups } was replaced to {{ alloy_service_group }} instead of {{ [ alloy_service_group ] + alloy_user_groups }}

I added MR 276.

Thanks in advance

pjezek avatar Oct 02 '24 09:10 pjezek