[pre-commit][kuttl]Fix false positive asserts #430

gibizer · 2024-05-13T12:31:12Z

Kuttl only runs the last TestAssert in an assert file and silently
ignores the rest. So this patch combines the TestAsserts into a single
one with multiple commands listed.
Also to further secure against false positives set -euxo pipefail is
added to the scripts as well.
Also added a new pre-commit check to avoid this in the future.

gibizer · 2024-05-13T13:21:31Z

Here is an example that before this patch only the last TestAssert was run in the tls test:
https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openstack-k8s-operators_ironic-operator/427/pull-ci-openstack-k8s-operators-ironic-operator-main-ironic-operator-build-deploy-kuttl/1785252428715659264/artifacts/ironic-operator-build-deploy-kuttl/openstack-k8s-operators-kuttl/build-log.txt

    logger.go:42: 11:01:26 | deploy_tls/10-deploy-ironic | running command: [sh -c template='{{.spec.endpoints.internal}}{{":"}}{{.spec.endpoints.public}}{{"\n"}}'
        regex="https:\/\/ironic-inspector-internal.$NAMESPACE.*:https:\/\/ironic-inspector-public.$NAMESPACE.*"
        apiEndpoints=$(oc get -n $NAMESPACE KeystoneEndpoint ironic-inspector -o go-template="$template")
        matches=$(echo "$apiEndpoints" | sed -e "s?$regex??")
        if [[ -n "$matches" ]]; then
          exit 1
        fi
        ]
    logger.go:42: 11:01:27 | deploy_tls/10-deploy-ironic | test step completed 10-deploy-ironic

gibizer · 2024-05-13T16:23:19Z

now kuttl tries to run all the asserts but the first one that wasn't running before fails:

    logger.go:42: 13:10:00 | deploy/15- | starting test step 15-
    logger.go:42: 13:10:00 | deploy/15- | running command: [sh -c set -euxo pipefail
        template='{{.status.apiEndpoints.ironic.public}}{{":"}}{{.status.apiEndpoints.ironic.internal}}{{"\n"}}'
        regex="http:\/\/ironic-public-$NAMESPACE\.apps.*:http:\/\/ironic-admin-$NAMESPACE\.apps.*:http:\/\/ironic-internal-$NAMESPACE\.apps.*"
        apiEndpoints=$(oc get -n $NAMESPACE ironics.ironic.openstack.org ironic -o go-template="$template")
        matches=$(echo "$apiEndpoints" | sed -e "s?$regex??")
        if [ -z "$matches" ]; then
          exit 0
        else
          exit 1
        fi
        ]
    logger.go:42: 13:10:00 | deploy/15- | + template='{{.status.apiEndpoints.ironic.public}}{{":"}}{{.status.apiEndpoints.ironic.internal}}{{"\n"}}'
    logger.go:42: 13:10:00 | deploy/15- | + regex='http:\/\/ironic-public-ironic-kuttl-tests\.apps.*:http:\/\/ironic-admin-ironic-kuttl-tests\.apps.*:http:\/\/ironic-internal-ironic-kuttl-tests\.apps.*'
    logger.go:42: 13:10:00 | deploy/15- | ++ oc get -n ironic-kuttl-tests ironics.ironic.openstack.org ironic -o 'go-template={{.status.apiEndpoints.ironic.public}}{{":"}}{{.status.apiEndpoints.ironic.internal}}{{"\n"}}'
    logger.go:42: 13:10:00 | deploy/15- | + apiEndpoints=http://ironic-public.ironic-kuttl-tests.svc:6385:http://ironic-internal.ironic-kuttl-tests.svc:6385
    logger.go:42: 13:10:00 | deploy/15- | ++ echo http://ironic-public.ironic-kuttl-tests.svc:6385:http://ironic-internal.ironic-kuttl-tests.svc:6385
    logger.go:42: 13:10:00 | deploy/15- | ++ sed -e 's?http:\/\/ironic-public-ironic-kuttl-tests\.apps.*:http:\/\/ironic-admin-ironic-kuttl-tests\.apps.*:http:\/\/ironic-internal-ironic-kuttl-tests\.apps.*??'
    logger.go:42: 13:10:00 | deploy/15- | + matches=http://ironic-public.ironic-kuttl-tests.svc:6385:http://ironic-internal.ironic-kuttl-tests.svc:6385
    logger.go:42: 13:10:00 | deploy/15- | + '[' -z http://ironic-public.ironic-kuttl-tests.svc:6385:http://ironic-internal.ironic-kuttl-tests.svc:6385 ']'
    logger.go:42: 13:10:00 | deploy/15- | + exit 1
    logger.go:42: 13:10:00 | deploy/15- | command failure, skipping 4 additional commands

https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/pr-logs/pull/openstack-k8s-operators_ironic-operator/430/pull-ci-openstack-k8s-operators-ironic-operator-main-ironic-operator-build-deploy-kuttl/1789996831153852416/artifacts/ironic-operator-build-deploy-kuttl/openstack-k8s-operators-kuttl/build-log.txt

I'm not 100% sure but it feels like the pattern matching logic is incorrect in the test as there are matches for the endpoint pattern but the script consider that as an error.

I need help here from @steveb or others from ironic.

steveb · 2024-05-14T00:24:18Z

It looks like .apps is no longer part of the endpoint host name, I'll take over this PR to get it green again. Thanks

gibizer · 2024-05-14T07:23:03Z

It looks like .apps is no longer part of the endpoint host name, I'll take over this PR to get it green again. Thanks

thank you!

softwarefactory-project-zuul · 2024-05-15T02:28:04Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/0439c7d5e31346ca9b3b97e3d4d9c7ac

✔️ noop SUCCESS in 0s
❌ openstack-k8s-operators-content-provider FAILURE in 10m 03s
⚠️ podified-multinode-ironic-deployment SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

softwarefactory-project-zuul · 2024-05-15T02:51:04Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/a818be68eb5e490ba495bf1d8ef3a158

✔️ noop SUCCESS in 0s
❌ openstack-k8s-operators-content-provider FAILURE in 10m 30s
⚠️ podified-multinode-ironic-deployment SKIPPED Skipped due to failed job openstack-k8s-operators-content-provider

softwarefactory-project-zuul · 2024-05-16T02:29:39Z

Build failed (check pipeline). Post recheck (without leading slash)
to rerun all jobs. Make sure the failure cause has been resolved before
you rerun jobs.

https://review.rdoproject.org/zuul/buildset/53f7d227c85e4dcdbc852156eb50cbde

✔️ noop SUCCESS in 0s
✔️ openstack-k8s-operators-content-provider SUCCESS in 58m 19s
❌ podified-multinode-ironic-deployment FAILURE in 34m 10s

steveb · 2024-05-21T23:33:14Z

/test ironic-operator-build-deploy-kuttl

steveb · 2024-05-22T02:55:42Z

Current failure is the public ironic endpoint not returning a status code:

    logger.go:42: 00:33:33 | standalone/15- | + template='{{index .status.apiEndpoints "ironic-inspector" "public"}}{{":"}}{{index .status.apiEndpoints "ironic-inspector" "internal"}}{{"\n"}}'
    logger.go:42: 00:33:33 | standalone/15- | + regex='http:\/\/ironic-inspector-public\.ironic-kuttl-tests\..*:http:\/\/ironic-inspector-internal\.ironic-kuttl-tests\..*'
    logger.go:42: 00:33:33 | standalone/15- | ++ oc get -n ironic-kuttl-tests ironics.ironic.openstack.org ironic -o 'go-template={{index .status.apiEndpoints "ironic-inspector" "public"}}{{":"}}{{index .status.apiEndpoints "ironic-inspector" "internal"}}{{"\n"}}'
    logger.go:42: 00:33:34 | standalone/15- | + apiEndpoints=http://ironic-inspector-public.ironic-kuttl-tests.svc:5050:http://ironic-inspector-internal.ironic-kuttl-tests.svc:5050
    logger.go:42: 00:33:34 | standalone/15- | ++ echo http://ironic-inspector-public.ironic-kuttl-tests.svc:5050:http://ironic-inspector-internal.ironic-kuttl-tests.svc:5050
    logger.go:42: 00:33:34 | standalone/15- | ++ sed -e 's?http:\/\/ironic-inspector-public\.ironic-kuttl-tests\..*:http:\/\/ironic-inspector-internal\.ironic-kuttl-tests\..*??'
    logger.go:42: 00:33:34 | standalone/15- | + matches=
    logger.go:42: 00:33:34 | standalone/15- | + '[' -z '' ']'
    logger.go:42: 00:33:34 | standalone/15- | + exit 0
    logger.go:42: 00:33:34 | standalone/15- | running command: [sh -c set -euxo pipefail
        RETURN_CODE=0
        PUBLIC_URL=$(oc get -n $NAMESPACE ironics.ironic.openstack.org ironic -o jsonpath='{.status.apiEndpoints.ironic.public}')
        if [ -z "$PUBLIC_URL" ]; then
            RETURN_CODE=1
            echo "Endpoint: apiEndpoints.ironic.public not ready."
            sleep 10
        else
            STATUSCODE=$(curl --silent --output /dev/stderr --head --write-out "%{http_code}" $PUBLIC_URL)
            if test $STATUSCODE -ne 200; then
                RETURN_CODE=1
                echo "${PUBLIC_URL} status code expected is 200 but was ${STATUSCODE}"
            fi
        fi
        exit $RETURN_CODE
        ]
    logger.go:42: 00:33:34 | standalone/15- | + RETURN_CODE=0
    logger.go:42: 00:33:34 | standalone/15- | ++ oc get -n ironic-kuttl-tests ironics.ironic.openstack.org ironic -o 'jsonpath={.status.apiEndpoints.ironic.public}'
    logger.go:42: 00:33:34 | standalone/15- | + PUBLIC_URL=http://ironic-public.ironic-kuttl-tests.svc:6385
    logger.go:42: 00:33:34 | standalone/15- | + '[' -z http://ironic-public.ironic-kuttl-tests.svc:6385 ']'
    logger.go:42: 00:33:34 | standalone/15- | ++ curl --silent --output /dev/stderr --head --write-out '%{http_code}' http://ironic-public.ironic-kuttl-tests.svc:6385
    logger.go:42: 00:33:34 | standalone/15- | + STATUSCODE=000
    logger.go:42: 00:33:34 | standalone/15- | command failure, skipping 2 additional commands
    logger.go:42: 00:33:35 | standalone/15- | test step failed 15-

steveb · 2024-06-21T00:34:32Z

/retest

Kuttl only runs the last TestAssert in an assert file and silently ignores the rest. So this patch combines the TestAsserts into a single one with multiple commands listed. Also to further secure against false positives `set -euxo pipefail` is added to the scripts as well.

Add a new pre-commit check to avoid using more than one TestkAssert in a single kuttl assert file as that can lead to false positives as only the last TestAssert is run by kuttl. The false positive tests fixed in a separate commit.

These tests were not running, and need fixing either due to changes in endpoint names, or incorrect regex patterns.

Public endpoint names won't resolve outside the cluster without /etc/hosts entries or integrated DNS, neither of which are occuring in the kuttl jobs. So the endpoint test needs to occur inside the cluster, which is achieved by doing an "oc run" with the official curl container image.

This means the -n argument is not required repeatedly for "oc get" and also "oc run" calls run in the expected namespace.

steveb · 2024-06-25T23:14:27Z

/retest

openshift-ci · 2024-06-26T18:43:52Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gibizer, hjensas

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [hjensas]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot requested review from abays and viroel May 13, 2024 12:31

gibizer changed the title ~~kuttl multiple test asserts~~ [pre-commit][kuttl]Fix false positive asserts May 13, 2024

steveb force-pushed the kuttl-multiple-test-asserts branch 3 times, most recently from 9bcebfa to 57fdd7d Compare May 16, 2024 01:29

steveb force-pushed the kuttl-multiple-test-asserts branch from 57fdd7d to 58c4c8e Compare June 21, 2024 03:10

steveb changed the title ~~[pre-commit][kuttl]Fix false positive asserts~~ [DNM][pre-commit][kuttl]Fix false positive asserts Jun 21, 2024

gibizer and others added 4 commits June 24, 2024 16:09

[pre-commit][kuttl]Check for multiple TestAsserts

bbe5cd4

Add a new pre-commit check to avoid using more than one TestkAssert in a single kuttl assert file as that can lead to false positives as only the last TestAssert is run by kuttl. The false positive tests fixed in a separate commit.

Fix all kuttl asserts

9338ee9

These tests were not running, and need fixing either due to changes in endpoint names, or incorrect regex patterns.

steveb force-pushed the kuttl-multiple-test-asserts branch from 6fd0453 to 003e6d2 Compare June 24, 2024 04:53

Set namespace context to $NAMESPACE

c197299

This means the -n argument is not required repeatedly for "oc get" and also "oc run" calls run in the expected namespace.

steveb requested review from hjensas and juliakreger June 26, 2024 00:41

steveb changed the title ~~[DNM][pre-commit][kuttl]Fix false positive asserts~~ [pre-commit][kuttl]Fix false positive asserts Jun 26, 2024

hjensas approved these changes Jun 26, 2024

View reviewed changes

openshift-ci bot assigned hjensas Jun 26, 2024

openshift-ci bot added the lgtm label Jun 26, 2024

openshift-ci bot added the approved label Jun 26, 2024

openshift-merge-bot bot merged commit d5571e8 into openstack-k8s-operators:main Jun 26, 2024
7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pre-commit][kuttl]Fix false positive asserts #430

[pre-commit][kuttl]Fix false positive asserts #430

gibizer commented May 13, 2024 •

edited

Loading

gibizer commented May 13, 2024

gibizer commented May 13, 2024

steveb commented May 14, 2024

gibizer commented May 14, 2024

softwarefactory-project-zuul bot commented May 15, 2024

softwarefactory-project-zuul bot commented May 15, 2024

softwarefactory-project-zuul bot commented May 16, 2024

steveb commented May 21, 2024

steveb commented May 22, 2024

steveb commented Jun 21, 2024

steveb commented Jun 25, 2024

openshift-ci bot commented Jun 26, 2024

[pre-commit][kuttl]Fix false positive asserts #430

[pre-commit][kuttl]Fix false positive asserts #430

Conversation

gibizer commented May 13, 2024 • edited Loading

gibizer commented May 13, 2024

gibizer commented May 13, 2024

steveb commented May 14, 2024

gibizer commented May 14, 2024

softwarefactory-project-zuul bot commented May 15, 2024

softwarefactory-project-zuul bot commented May 15, 2024

softwarefactory-project-zuul bot commented May 16, 2024

steveb commented May 21, 2024

steveb commented May 22, 2024

steveb commented Jun 21, 2024

steveb commented Jun 25, 2024

openshift-ci bot commented Jun 26, 2024

gibizer commented May 13, 2024 •

edited

Loading