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

Fix: Routed logs test now correctly checks if CNF is being tailed #2034

Merged
merged 3 commits into from
Jun 10, 2024

Conversation

svteb
Copy link
Collaborator

@svteb svteb commented May 16, 2024

Description

Fixed two issues regarding routed_logs and fluent family of functions as described in referenced issues. Nearly everything was refactored, but all the install/uninstall and tailing functions have been manually tested and the spec tests are passing as well. Main reasons for the refactor were shortening the code and removing duplicates.

As for the refactors, the CONFIG variable does not look all that pretty but I could not figure out a better way to achieve the modularity.

Issues:

Refs: #2025 #2016

How has this been tested:

  • Covered by existing integration testing
  • Added integration testing to cover
  • Verified all A/C passes
    • develop
    • master
    • tag/other branch
  • Test environment
    • Shared Packet K8s cluster
    • New Packet K8s cluster
    • Kind cluster
    • Minikube cluster
  • Have not tested

Types of changes:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist:

Documentation

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • No updates required.
  • Maybe, I'll have to check.

Code Review

  • Does the test handle fatal exceptions, ie. rescue block

Issue

  • Tasks in issue are checked off

Ref: cnti-testcatalog#2025 cnti-testcatalog#2016
- The issue explained in cnti-testcatalog#2025 where fluent was tailing itself
instead of the CNF has been resolved.
- The dead helm repo mentioned in cnti-testcatalog#2016 has been replace by link to bitnami repo,
there was a similar occurence in the spec test.
- The family of fluentd functions and files have been refactored into configurable
files (allows for future additions/checking of elastic, aws fluent, etc.).
- The routed_logs test was also modified to conform to the new functions (also shortened).
- Other files have been edited for completion/conformance with naming conventions.
Signed-off-by: svteb <slavo.valko@tietoevry.com>
@taylor
Copy link
Collaborator

taylor commented May 16, 2024

re-running specs after runners restored

svteb added 2 commits May 17, 2024 07:25
Refs: cnti-testcatalog#2025

Signed-off-by: svteb <slavo.valko@tietoevry.com>
Refs: cnti-testcatalog#2025
- Reworked the config dictionary into classes that inherit
from a base abstract class.
- Renamed FluentManagement module to FluentManager
- Interfaces changed to adhere to the new abstraction

Signed-off-by: svteb <slavo.valko@tietoevry.com>
@martin-mat martin-mat self-requested a review June 3, 2024 14:36
Copy link
Collaborator

@martin-mat martin-mat left a comment

Choose a reason for hiding this comment

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

lgtm

@horecoli
Copy link
Contributor

horecoli commented Jun 7, 2024

LGTM, good job 👍

@martin-mat
Copy link
Collaborator

@martin-mat martin-mat merged commit 310e9c7 into cnti-testcatalog:main Jun 10, 2024
14 of 88 checks passed
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.

4 participants