fix(discovery): fix broken DISABLE_BUILTIN_DISCOVERY env var #1506

andrewazores · 2023-05-30T20:03:25Z

Welcome to Cryostat! 👋

Before contributing, make sure you have:

Read the contributing guidelines
Linked a relevant issue which this PR resolves
Linked any other relevant issues, PR's, or documentation, if any
Resolved all conflicts, if any
Rebased your branch PR on top of the latest upstream main branch
Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
Signed the last commit: git commit --amend --signoff

Description of the change:

This does some refactoring to ensure that the Custom Targets functionality is always available, even when the environment variable CRYOSTAT_DISABLE_BUILTIN_DISCOVERY is set, and so that setting that environment variable does not result in a startup-blocking exception.

Motivation for the change:

It may be useful in some situations to disable the built-in discovery mechanisms entirely. A Cryostat instance locked down in this way can be sure to never discover any target applications other than ones registered manually using Custom Targets, or ones configured using the Cryostat Agent.

How to manually test:

CRYOSTAT_IMAGE=quay.io... CRYOSTAT_DISABLE_BUILTIN_DISCOVERY=true sh smoketest.sh
Verify that the container started successfully
https :8181/api/v2.1/discovery to ensure that only the Custom Targets Realm is reported

tthvo · 2023-05-30T20:50:25Z

Code looks good but I am not sure something is missing because the built-in discovery is still enabled with (using image built from this PR commit):

CRYOSTAT_DISABLE_BUILTIN_DISCOVERY=true sh smoketest.sh

now that multiple platforms can be used in parallel, they do not need to have a priority for checking availability

andrewazores · 2023-05-30T20:58:36Z

$ CRYOSTAT_DISABLE_BUILTIN_DISCOVERY=true sh smoketest.sh

$ https --auth-type=basic --auth=user:pass :8181/api/v2.1/discovery
HTTP/1.1 200 OK
content-encoding: gzip
content-length: 999
content-type: application/json

{
    "data": {
        "result": {
            "children": [
                {
                    "children": [
                        {
                            "id": -432439206,
                            "labels": {},
                            "name": "http://localhost:8910/",
                            "nodeType": "JVM",
                            "target": {
                                "alias": "vertx-fib-demo-1",
                                "annotations": {
                                    "cryostat": {
                                        "HOST": "cryostat",
                                        "JAVA_MAIN": "es.andrewazor.demo.Main -javaagent:/opt/jib-agents/cryostat-agent-0.2.2.jar",
                                        "PID": "1",
                                        "PORT": "8910",
                                        "REALM": "vertx-fib-demo-1",
                                        "START_TIME": "1685480129"
                                    },
                                    "platform": {}
                                },
                                "connectUrl": "http://localhost:8910/",
                                "jvmId": "OzsZcRCrIZo00q0VvVX_j_7XwBHQfzqw5MNmMiXSolc=",
                                "labels": {}
                            }
                        }
                    ],
                    "id": 622503342,
                    "labels": {
                        "REALM": "db441505-c3be-44b0-8be5-63c416ae9323"
                    },
                    "name": "vertx-fib-demo-1",
                    "nodeType": "Realm"
                },
                {
                    "children": [
                        {
                            "id": -2088431488,
                            "labels": {},
                            "name": "http://localhost:8911/",
                            "nodeType": "JVM",
                            "target": {
                                "alias": "vertx-fib-demo-2",
                                "annotations": {
                                    "cryostat": {
                                        "HOST": "cryostat",
                                        "JAVA_MAIN": "es.andrewazor.demo.Main -javaagent:/opt/jib-agents/cryostat-agent-0.2.2.jar",
                                        "PID": "1",
                                        "PORT": "8911",
                                        "REALM": "vertx-fib-demo-2",
                                        "START_TIME": "1685480129"
                                    },
                                    "platform": {}
                                },
                                "connectUrl": "http://localhost:8911/",
                                "jvmId": "M_nb6vAVZWFzpLF140qloFnzhYuOsWc-fS2nMWG0NQ4=",
                                "labels": {}
                            }
                        }
                    ],
                    "id": 1613802238,
                    "labels": {
                        "REALM": "a17a77ad-9ae3-4f14-97ba-b7c32e851e8a"
                    },
                    "name": "vertx-fib-demo-2",
                    "nodeType": "Realm"
                },
                {
                    "children": [
                        {
                            "id": 813146660,
                            "labels": {},
                            "name": "http://localhost:8912/",
                            "nodeType": "JVM",
                            "target": {
                                "alias": "vertx-fib-demo-3",
                                "annotations": {
                                    "cryostat": {
                                        "HOST": "cryostat",
                                        "JAVA_MAIN": "es.andrewazor.demo.Main -javaagent:/opt/jib-agents/cryostat-agent-0.2.2.jar",
                                        "PID": "1",
                                        "PORT": "8912",
                                        "REALM": "vertx-fib-demo-3",
                                        "START_TIME": "1685480129"
                                    },
                                    "platform": {}
                                },
                                "connectUrl": "http://localhost:8912/",
                                "jvmId": "idNwhKBD4Ow2iNZi3RHeqU_CMFTJrJhJmGQ3drFavTc=",
                                "labels": {}
                            }
                        }
                    ],
                    "id": 1754892022,
                    "labels": {
                        "REALM": "d14a044f-f794-4f2d-84a5-b31a333261e8"
                    },
                    "name": "vertx-fib-demo-3",
                    "nodeType": "Realm"
                },
                {
                    "children": [
                        {
                            "id": -163233355,
                            "labels": {},
                            "name": "service:jmx:rmi:///jndi/rmi://cryostat:9097/jmxrmi",
                            "nodeType": "JVM",
                            "target": {
                                "alias": "quarkus-test-agent",
                                "annotations": {
                                    "cryostat": {
                                        "HOST": "cryostat",
                                        "JAVA_MAIN": "/deployments/quarkus-run.jar",
                                        "PID": "1",
                                        "PORT": "-1",
                                        "REALM": "quarkus-test-agent",
                                        "START_TIME": "1685480129"
                                    },
                                    "platform": {}
                                },
                                "connectUrl": "service:jmx:rmi:///jndi/rmi://cryostat:9097/jmxrmi",
                                "jvmId": "kDJLGwATFCHKY_sTWZM_9vZ_mmBnNNGMvS_0_54FJB0=",
                                "labels": {}
                            }
                        }
                    ],
                    "id": 1827072391,
                    "labels": {
                        "REALM": "1b8c77ad-6e8f-44ab-8bba-bf4eac1f8372"
                    },
                    "name": "quarkus-test-agent",
                    "nodeType": "Realm"
                },
                {
                    "children": [
                        {
                            "id": -1576634802,
                            "labels": {},
                            "name": "http://localhost:9988/",
                            "nodeType": "JVM",
                            "target": {
                                "alias": "quarkus-test-agent",
                                "annotations": {
                                    "cryostat": {
                                        "HOST": "cryostat",
                                        "JAVA_MAIN": "/deployments/quarkus-run.jar",
                                        "PID": "1",
                                        "PORT": "9988",
                                        "REALM": "quarkus-test-agent",
                                        "START_TIME": "1685480130"
                                    },
                                    "platform": {}
                                },
                                "connectUrl": "http://localhost:9988/",
                                "jvmId": "do5_f7fTao7uU4V80FYlFt62RuMhiDaSIHmAZ_771Jw=",
                                "labels": {}
                            }
                        }
                    ],
                    "id": 1487758365,
                    "labels": {
                        "REALM": "54a8f2b1-3dbf-448c-bb37-e02cd45fcc58"
                    },
                    "name": "quarkus-test-agent",
                    "nodeType": "Realm"
                },
                {
                    "children": [],
                    "id": -511444528,
                    "labels": {
                        "REALM": "add51184-c47d-4d72-8131-41bab315802c"
                    },
                    "name": "Custom Targets",
                    "nodeType": "Realm"
                }
            ],
            "id": 1171029256,
            "labels": {},
            "name": "Universe",
            "nodeType": "Universe"
        }
    },
    "meta": {
        "status": "OK",
        "type": "application/json"
    }
}

The latest commit does some cleanup which should also fix a conflict between Custom Targets and JDP that existed before. Though, I don't think that should have caused the builtins to still appear for you.

tthvo · 2023-05-30T21:02:46Z

Hmm weird. I built the image with mvn clean package -DskipTests=true and ran the same scripts :((

andrewazores · 2023-05-30T21:03:54Z

Maybe try sh smoketest.sh h2mem? Maybe the old entries are just hanging out in the database still?

tthvo · 2023-05-30T21:06:10Z

Ohh totally forgot about that again. Worked fine with h2mem. We don't yet have any handler to prune discovery database right?

andrewazores · 2023-05-30T21:09:51Z

I'll give it some thought. I think what I should do is set it up so that any disabled builtins have their subtrees automatically pruned from the discovery tree, ie drop the whole Realm. There is no ongoing whole-tree pruning that happens, that is expected to be done by each plugin at the Realm level, or by the plugins themselves registering and deregistering (or failing to respond to heartbeat pings).

tthvo · 2023-05-30T21:15:54Z

I think what I should do is set it up so that any disabled builtins have their subtrees automatically pruned from the discovery tree, ie drop the whole Realm.

Sounds good! Makes sense to me! I guess other things just gonna be intergration test is failing as it expects more targets than actual.

andrewazores · 2023-05-31T13:42:28Z

It looks like I already thought of that mechanism:

        unselectedStrategies.stream()
                .map(PlatformDetectionStrategy::getPlatformClient)
                .forEach(
                        platform ->
                                storage.getBuiltInPluginByRealm(
                                                platform.getDiscoveryTree().getName())
                                        .map(PluginInfo::getId)
                                        .ifPresent(storage::deregister));

That's in the startup for the BuiltInDiscovery. Not sure why it didn't work.

github-actions · 2023-05-31T14:43:10Z

Test image available:

$ CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat:pr-1506-f60092314eb66e0cd9d5254c54134468b8773d7c sh smoketest.sh

andrewazores · 2023-05-31T14:47:02Z

That's in the startup for the BuiltInDiscovery. Not sure why it didn't work.

The last refactoring probably fixed this, and also fixed the broken integration test. The earlier refactoring I did in this PR broke some of how the DI setup was put together, so things were not all @Singleton as they should have been, so the determination of which platforms were selected and which were not selected was broken.

github-actions · 2023-05-31T15:47:02Z

Test image available:

$ CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat:pr-1506-5cb0e49b5c09e70cdaf71201184df76b754c0f7a sh smoketest.sh

tthvo

Looks good to me! Fixed the issue!

* fix(discovery): fix broken DISABLE_BUILT_DISCOVERY env var * remove no longer necessary 'priority' for platforms (cherry picked from commit dda9672)

…1509) * fix(discovery): fix broken DISABLE_BUILT_DISCOVERY env var * remove no longer necessary 'priority' for platforms (cherry picked from commit dda9672) Co-authored-by: Andrew Azores <aazores@redhat.com>

fix(discovery): fix broken DISABLE_BUILT_DISCOVERY env var

5799740

andrewazores added fix backport labels May 30, 2023

mergify bot added the safe-to-test label May 30, 2023

andrewazores requested a review from tthvo May 30, 2023 20:04

andrewazores changed the title ~~fix(discovery): fix broken DISABLE_BUILT_DISCOVERY env var~~ fix(discovery): fix broken DISABLE_BUILTIN_DISCOVERY env var May 30, 2023

remove no longer necessary 'priority' for platforms

a53756d

now that multiple platforms can be used in parallel, they do not need to have a priority for checking availability

refactoring

f600923

andrewazores added 3 commits May 31, 2023 11:08

remove logger lines

5909f8a

fixup! remove logger lines

ba23a62

minor refactor

5cb0e49

tthvo approved these changes May 31, 2023

View reviewed changes

andrewazores merged commit dda9672 into cryostatio:main Jun 1, 2023

andrewazores deleted the disabled-discovery-bug branch June 1, 2023 13:05

mergify bot mentioned this pull request Jun 1, 2023

fix(discovery): fix broken DISABLE_BUILTIN_DISCOVERY env var (backport #1506) #1509

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(discovery): fix broken DISABLE_BUILTIN_DISCOVERY env var #1506

fix(discovery): fix broken DISABLE_BUILTIN_DISCOVERY env var #1506

andrewazores commented May 30, 2023

tthvo commented May 30, 2023

andrewazores commented May 30, 2023

tthvo commented May 30, 2023

andrewazores commented May 30, 2023

tthvo commented May 30, 2023

andrewazores commented May 30, 2023

tthvo commented May 30, 2023

andrewazores commented May 31, 2023

github-actions bot commented May 31, 2023

andrewazores commented May 31, 2023

github-actions bot commented May 31, 2023

tthvo left a comment

fix(discovery): fix broken DISABLE_BUILTIN_DISCOVERY env var #1506

fix(discovery): fix broken DISABLE_BUILTIN_DISCOVERY env var #1506

Conversation

andrewazores commented May 30, 2023

Welcome to Cryostat! 👋

Before contributing, make sure you have:

Description of the change:

Motivation for the change:

How to manually test:

tthvo commented May 30, 2023

andrewazores commented May 30, 2023

tthvo commented May 30, 2023

andrewazores commented May 30, 2023

tthvo commented May 30, 2023

andrewazores commented May 30, 2023

tthvo commented May 30, 2023

andrewazores commented May 31, 2023

github-actions bot commented May 31, 2023

andrewazores commented May 31, 2023

github-actions bot commented May 31, 2023

tthvo left a comment

Choose a reason for hiding this comment