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.

Sign up for GitHub

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

Jump to bottom

fix: add support for Valkey #212

Merged
merged 4 commits into from
Oct 14, 2024
Merged

fix: add support for Valkey #212

merged 4 commits into from
Oct 14, 2024

Conversation

natoscott
Copy link
Collaborator

Enhancement: Support Valkey as an alternative to Redis

Reason: Redis is no longer open source and is being removed from distributions

Result: PCP scalable query language + Grafana support continue to function

Issue Tracker Tickets (Jira or BZ if any): RHEL-29939

@natoscott natoscott requested a review from richm as a code owner August 21, 2024 05:49
spetrosi
spetrosi reviewed Aug 21, 2024
View reviewed changes
tasks/firewall.yml Outdated Show resolved Hide resolved
tests/check_default_datasources.yml Outdated Show resolved Hide resolved
tests/check_keyserver.yml Outdated Show resolved Hide resolved
@spetrosi
Copy link
Contributor

[citest]

3 similar comments
@spetrosi
Copy link
Contributor

[citest]

@spetrosi
Copy link
Contributor

[citest]

@spetrosi
Copy link
Contributor

[citest]

@spetrosi
Copy link
Contributor

[citest]

@spetrosi
Copy link
Contributor

@natoscott there is an error chown failed: failed to look up user redis in ci tests, please take a look

richm
richm reviewed Sep 25, 2024
View reviewed changes
tasks/firewall.yml Outdated
@@ -21,7 +21,7 @@
__metrics_firewall: "{{ __metrics_firewall |
union([{'port': '44322/tcp', 'state': 'enabled'}]) }}"
when:
- metrics_graph_service|bool or metrics_query_service|bool
- metrics_graph_service | bool or metrics_query_service|bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- metrics_graph_service | bool or metrics_query_service|bool
- metrics_graph_service | bool or metrics_query_service | bool

richm
richm reviewed Sep 25, 2024
View reviewed changes
tasks/firewall.yml Outdated Show resolved Hide resolved
@natoscott
Copy link
Collaborator Author

@natoscott there is an error chown failed: failed to look up user redis in ci tests, please take a look

Will do. I have limited time currently, with CVEs, escalations and PTO - but I'll keep digging into this one as time is available (wont be this week) - thanks for all the reviews.

@natoscott
Copy link
Collaborator Author

[citest]

@natoscott
Copy link
Collaborator Author

[citest]

@natoscott natoscott force-pushed the valkey branch 2 times, most recently from 3c13ae2 to 22eddea Compare October 9, 2024 05:43
@natoscott
Copy link
Collaborator Author

[citest]

@natoscott
Copy link
Collaborator Author

[citest]

@spetrosi
Copy link
Contributor

[citest]

@spetrosi
Copy link
Contributor

@natoscott I fixed the CI-related issue with ansible not being able to find fedora.linux_system_roles.firewall in linux-system-roles/tft-tests#55, now there are some actual errors in CI run.

@richm
Copy link
Collaborator

richm commented Oct 14, 2024 

TASK [fedora.linux_system_roles.private_metrics_subrole_keyserver : Ensure key server is configured] ***
task path: /tmp/collections-EIz/ansible_collections/fedora/linux_system_roles/roles/private_metrics_subrole_keyserver/tasks/main.yml:46
Monday 14 October 2024  10:23:34 -0400 (0:00:00.560)       0:01:02.532 ******** 
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: ansible.errors.AnsibleUndefinedVariable: 'redis_save_to_disk' is undefined
failed: [managed-node2] (item=/tmp/collections-EIz/ansible_collections/fedora/linux_system_roles/roles/private_metrics_subrole_keyserver/templates/CentOS_9_keyserver.conf.j2) => {
    "ansible_loop_var": "item",
    "changed": false,
    "item": "/tmp/collections-EIz/ansible_collections/fedora/linux_system_roles/roles/private_metrics_subrole_keyserver/templates/CentOS_9_keyserver.conf.j2"
}

MSG:

AnsibleUndefinedVariable: 'redis_save_to_disk' is undefined

@natoscott
Squashed 'vendor/github.com/performancecopilot/ansible-pcp/' changes …
9173c3b 
…from ff37aa2..b59dee6

b59dee6 fix: convert remaining keyserver save_to_disk variables
b310a1f fix: resolve github actions warning about yaml doc start line

git-subtree-dir: vendor/github.com/performancecopilot/ansible-pcp
git-subtree-split: b59dee6d8ac02ae99d5b7707c67eedfdd7394476
@natoscott
Merge commit '9173c3bb834095dc253ab64a240c43071f6ebc3c' into valkey
5283457
@natoscott
Copy link
Collaborator Author

[citest]

@natoscott
Copy link
Collaborator Author

OK, the only remaining CI failure I can see now is "AnsibleUndefinedVariable: 'performancecopilot' is undefined" which is unrelated to this change. I think that might be related to ansible-pcp commit 9e4b26dbf66a which adds variables like "performancecopilot:ansible-pcp" but I'm not sure.

@natoscott natoscott merged commit 5283457 into linux-system-roles:main Oct 14, 2024
10 of 16 checks passed
@natoscott natoscott deleted the valkey branch October 15, 2024 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spetrosi spetrosi spetrosi left review comments

@richm richm richm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@natoscott @spetrosi @richm