[bonampak]

KNOX-2900 - Re-enable service-based discovery filter

What changes were proposed in this pull request?

PollingConfigurationAnalyzer changes:

• Ignore excluded service types in CM config monitor event relevance
• Ignore excluded role types in scale events
• Prevent rediscovery of services that remained in an invalid config state (use the same workflow for service configuration data discovery as ClouderaManagerServiceDiscovery)
• Re-enable per-descriptor service filtering in CM discovery (cluster configuration cache data is merged after each topology discovery)

How was this patch tested?

Unit tests, manual tests on a dev cluster.

Integration Tests

[github-actions]

Test Results

  4 files    4 suites   1m 28s ⏱️
145 tests 145 ✅ 0 💤 0 ❌
169 runs  169 ✅ 0 💤 0 ❌

Results for commit e69cd65.

♻️ This comment has been updated with latest results.

[smolnar82]

I found 2 blockers, and the following efficiency issues:

1. every descriptor is parsed twice per polling cycle

File: PollingConfigurationAnalyzer.java (getReferencedServiceTypes() / clusterReferencesExist())

clusterReferencesExist() and getReferencedServiceTypes() each independently loop over ts.getDescriptors() and re-parse every descriptor file via SimpleDescriptorFactory.parse() (around lines 423 and 602). With N descriptors on disk, each polling cycle now parses every file twice per cluster — doubling filesystem reads + JSON parsing — and duplicates near-identical IOException handling.

Fix direction: merge the two into a single pass over ts.getDescriptors() that returns both "does any descriptor reference this (address, cluster)" and "the set of referenced service names", so each descriptor is parsed once.

2. CORE_SETTINGS is re-fetched from CM once per relevant service type

File: PollingConfigurationAnalyzer.java (getCoreSettingsConfig())

getCoreSettingsConfig() issues a fresh readServices(clusterName, summary) plus a readServiceConfig() round-trip every time getCurrentServiceConfiguration() runs — i.e. once per distinct relevant service type per polling cycle. Cluster discovery, by contrast, computes CORE_SETTINGS once for the whole cluster (coreSettingsConfig(...)).

Failure path: a polling cycle with events for three service types (e.g. HDFS, HIVE_ON_TEZ, YARN) makes three separate readServices() + readServiceConfig() calls just to re-derive the same CORE_SETTINGS value, adding avoidable CM API load on every interval.

Fix direction: compute CORE_SETTINGS once per polling pass and reuse it across all services handled in that pass (cache it for the duration of the cycle).

3.: role-type exclusion config is cached for the analyzer's lifetime while service-type exclusion is read fresh

File: PollingConfigurationAnalyzer.java (isExcludedRoleType() / isExcludedServiceType())

isExcludedRoleType() lazily builds and then permanently caches its TypeNameFilter (roleTypeFilter) from gatewayConfig on first use, whereas the adjacent isExcludedServiceType() re-reads gatewayConfig.getClouderaManagerServiceDiscoveryExcludedServiceTypes() fresh on every call.

Failure path: an operator updates gateway.cloudera.manager.service.discovery.excluded.role.types on a running gateway (without restarting the CM configuration monitor thread). isExcludedServiceType honors an equivalent excluded-service-types change immediately, but isExcludedRoleType keeps using the stale filter built at first invocation and silently ignores the update.

Fix direction: pick one policy for both. Either read both config values fresh per call, or resolve both once at construction — but don't split the behavior between the two sibling checks introduced in this PR.

[smolnar82]

To reuse the discovery workflow, getCurrentServiceConfiguration() reconstructs the ApiService / ApiRole / config objects it feeds into ServiceModelFactory.generateServiceModels(). But this reconstruction doesn't populate the same fields that real discovery (ClouderaManagerServiceDiscovery.discoverService) hands to generators. Any ServiceModelGenerator that dereferences a field the reconstruction leaves null throws an NPE inside generateService(). This is not an edge case — it hits the majority of generators:

• role.getHostRef().getHostname() — used by 34 of 51 generators (OozieServiceModelGenerator, SolrServiceModelGenerator, HBase, Impala, and most others). NPEs if the reconstructed role's hostRef (or its hostname) isn't populated exactly as discovery populates it.
• service.getClusterRef().getClusterName() — used by YarnUIServiceModelGenerator and JobHistoryUIServiceModelGenerator. NPEs because the synthetic ApiService sets only name / type, no clusterRef.
• Any other field a given generator's handles() / generateService() reads that the reconstruction omits.

Failure path & blast radius: a start/restart/scale event for almost any discoverable service (YARN, Oozie, Solr, HBase, …) → hasConfigChanged → getCurrentServiceConfiguration → ServiceModelFactory.generateServiceModels → the generator's generateService() → NPE. The NPE is not an ApiException, so the method's own try/catch doesn't catch it; it propagates to monitorClusterConfigurationChanges' outer catch(Exception), aborting the entire polling cycle for all clusters. Because the triggering event is never marked processed, the same NPE recurs every polling interval — PCA is effectively dead for that gateway.

Root cause / fix direction: the reconstruction in getCurrentServiceConfiguration() diverges from how ClouderaManagerServiceDiscovery builds these objects. Rather than hand-rebuilding ApiService / ApiRole from readServiceConfig, PCA should obtain the model inputs through the same code path discovery uses (the shared component introduced here), so every field a generator may read is populated identically. A unit test that runs each registered generator against PCA-reconstructed inputs would have caught this and would guard against regressions as new generators land.

[bonampak]

Fixed:

• 8c06e4d: fixes the clusterRef NPE by passing the real ApiService from the cluster's service list.
• 0561951: adds the test that runs every registered generator against monitor-shaped inputs.
• dd30643: adds the RuntimeException guard so a failing generator no longer ends the whole polling cycle, and tightens its test.

For hostRef, the shared ServiceModelFactory from 7cc335a already covered it.

[smolnar82]

This new branch treats getCurrentServiceConfiguration() == null as "the service is now invalid / disabled" and forces re-discovery. But that method also returns null on any ApiException - network blip, auth failure, transient 5xx:

} catch (ApiException e) {
  log.clouderaManagerConfigurationAPIError(e);
}
return currentConfig; // still null on API error

So a null return is ambiguous: it can mean either "config genuinely produced no model" or "couldn't reach CM". hasConfigChanged's else branch (prior config present, currentConfig == null) can't tell them apart and unconditionally sets configHasChanged = true, logs serviceDisabled, and triggers a full re-discovery.

Failure path: CM API has a transient outage while computing the current config for a service that has a valid prior baseline → getCurrentServiceConfiguration catches the ApiException and returns null → hasConfigChanged forces re-discovery → repeats every polling cycle until CM recovers.

Regression: previously an API-error null was a harmless no-op (configHasChanged stayed false). This PR turns a transient CM error into a repeating full-cluster rediscovery storm — the same failure class KNOX-2900 set out to eliminate.

Fix direction: distinguish "no model produced" from "CM unreachable". Options: let the ApiException propagate (or rethrow) instead of collapsing it to null, or return a tri-state / Optional so the caller can skip the change decision on API errors and only treat a genuinely empty-but-successful result as "service invalid".

[bonampak]

Fixed by

• 4b0c342 returns a three-state result so an unreachable CM no longer forces a rediscovery
• 37f71bf adds the tests.

One limitation, which also existed on master: after an error the event is still marked processed,
so a real change that coincides with an outage isn't retried until another event arrives. It is not a regression.
I'd create a follow-up ticket for that.

[bonampak]

About the 3 issues above:

1. Every descriptor is parsed twice per polling cycle

• 013fa34 fixes it
• 8a377a9 "Avoid returning null for unknown referenced service types" cleans up the merged result type and adds test coverage for the unavailable TopologyService path.

2. CORE_SETTINGS is re-fetched from CM once per relevant service type

• 12a6721 fetches the service list and CORE_SETTINGS once per cluster per cycle.
• 2dcde95 adds tests for the CORE_SETTINGS fetch and for the refresh on each cycle.

3. Role-type exclusion config is cached for the analyzer's lifetime while service-type exclusion is read fresh

• 87436d7 reads the excluded role types fresh on each call like the service types
• 8f3c820 adds tests that change each setting between two cycles.