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

ipatests: ignore nsslapd-accesslog-logbuffering WARN in healthcheck #7074

Closed
wants to merge 2 commits into from

Conversation

rcritten
Copy link
Contributor

Log buffering is disabled in the integration tests so we can have all the logs at the end. This is causing a warning to show in the 389-ds checks and causing tests to fail that expect all SUCCESS.

Add an exclude for this specific key so tests will pass again.

We may eventually want a more sophisiticated mechanism to handle excludes, or updating the config in general, but this is fine for now.

Fixes: https://pagure.io/freeipa/issue/9400

@rcritten rcritten added the ipa-4-11 Mark for backport to ipa 4.11 label Nov 13, 2023
Copy link
Contributor

@flo-renaud flo-renaud left a comment

Choose a reason for hiding this comment

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

Hi @rcritten
the tests test_integration/test_ipahealthcheck.py::TestIpaHealthCLI::test_severity, test_integration/test_ipahealthcheck.py::TestIpaHealthCLI::test_input_file, test_integration/test_ipahealthcheck.py::TestIpaHealthCLI::test_pki_healthcheck and test_integration/test_replica_promotion.py::TestHiddenReplicaPromotion::test_ipahealthcheck_hidden_replica also need to exclude DSCLE0004.

@rcritten
Copy link
Contributor Author

temp_commit is failing with known issues https://pagure.io/freeipa/issue/9478 and https://pagure.io/freeipa/issue/9400

@rcritten
Copy link
Contributor Author

rcritten commented Nov 13, 2023

test_commit3 is failing with https://pagure.io/freeipa/issue/9463 (ipa-ods-exporter crashing)

@rcritten rcritten added the ipa-4-10 Mark for backport to ipa 4.10 label Nov 14, 2023
@rcritten
Copy link
Contributor Author

Rebased to pull in fix for https://pagure.io/freeipa/issue/9460

@rcritten
Copy link
Contributor Author

The remaining issue for temp_commit appears to be fixed upstream in freeipa/freeipa-healthcheck#313

@flo-renaud
Copy link
Contributor

The failure in fedora-latest/temp_commit is not related to this PR but another regression in the test with ipa-healthcheck 0.15 (freeipa/freeipa-healthcheck@e69589d introduced new error messages).
The failure in fedora-latest/temp_commit3 is a known issue, https://pagure.io/freeipa/issue/9463

@rcritten
Copy link
Contributor Author

I'm working on upstream freeipa-healtcheck changes in coordiation with ipatests changes to get IdM CI passing.
I hope that with the latest copr build + ipatests changes the tests will be green.

@rcritten rcritten force-pushed the issue_9400 branch 2 times, most recently from 938e160 to 2e917a9 Compare November 15, 2023 02:24
@miskopo miskopo self-requested a review November 15, 2023 10:46
conf = host.get_file_contents(
'/etc/ipahealthcheck/ipahealthcheck.conf', encoding='utf-8'
)
conf += '[excludes]'
Copy link
Member

@miskopo miskopo Nov 15, 2023

Choose a reason for hiding this comment

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

If you wanted to check, if the section is already there and avoid failure or be able to add multiple exclusion, something like this would probably also work

    config_data = host.get_file_contents(config, encoding='utf-8')
    cfg = RawConfigParser()
    cfg.read_string(config_data)
    # add section "excludes" if it doesn't exist
    excludes_section = "excludes"
    if not cfg.has_section(excludes_section):
        cfg.add_section(excludes_section)
    # add options to the excludes section
    for option, value in excluded_checks:
        if not cfg.has_option(excludes_section, option):
            cfg.set(excludes_section, option, value)
# output to the original config file

or using tasks.remote_ini_file to avoid temp files and stuff

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's strange is this code worked over multiple runs yesterday, just not the last one I ran for the day.

Copy link
Contributor

Choose a reason for hiding this comment

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

Without update_packages=true, the 389-ds version is the one from the vagrant image and it was generated a long time ago (with 389-ds-base-2.3.2-2). With update-packages=true the test downloads the latest version from the updates repo (2.3.7-2), and the check for log buffering was added in 2.3.5.

Log buffering is disabled in the integration tests so we can have all
the logs at the end. This is causing a warning to show in the 389-ds
checks and causing tests to fail that expect all SUCCESS.

Add an exclude for this specific key so tests will pass again.

We may eventually want a more sophisiticated mechanism to handle
excludes, or updating the config in general, but this is fine for now.

Fixes: https://pagure.io/freeipa/issue/9400

Signed-off-by: Rob Crittenden <[email protected]>
ipa-healthcheck commit e69589d5 changed the output when a service
keytab is missing to not report the GSSAPI error but to report
that the keytab doesn't exist at all. This distinguishes from real
Kerberos issues like kvno.

Fixes: https://pagure.io/freeipa/issue/9482

Signed-off-by: Rob Crittenden <[email protected]>
@rcritten
Copy link
Contributor Author

All tests are green.

Link to test_ipahealthcheck.py::TestIpaHealthCheck
Link to test_ipahealthcheck.py::TestIpaHealthCLI
Link to test_replica_promotion.py::TestHiddenReplicaPromotion

I need to submit the related changes I made to freeipa-healthcheck into Fedora 38, 39 and rawhide.

@rcritten
Copy link
Contributor Author

Two issues are addressed in freeipa-healthcheck-0.16-2:

  • If dirsrv is down don't error out with a connection error when checking the status of a service
  • Disable the ipa-ods-exporter service check because of https://pagure.io/freeipa/issue/9463

@flo-renaud
Copy link
Contributor

@rcritten thanks for the patch, works for me. You can remove the temp commit and I will ACK when PRCI completes.

@flo-renaud flo-renaud added the ack Pull Request approved, can be merged label Nov 16, 2023
@rcritten rcritten added the pushed Pull Request has already been pushed label Nov 16, 2023
@rcritten
Copy link
Contributor Author

master:

  • d659d21 ipatests: ignore nsslapd-accesslog-logbuffering WARN in healthcheck
  • f00b52c ipatests: fix expected output for ipahealthcheck.ipa.host

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged ipa-4-10 Mark for backport to ipa 4.10 ipa-4-11 Mark for backport to ipa 4.11 pushed Pull Request has already been pushed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants