Issue with replicaServiceTemplate in ClickHouseKeeperInstallation - Services not appearing

[carloss4dv]

Hi again !!!

Apologies for opening a new issue, but I believe this might be important. This problem comes from the last two messages in issue #1737.

Summary: When using replicaServiceTemplate instead of clusterServiceTemplate in a ClickHouseKeeperInstallation to apply Cilium annotations, the expected services (e.g., chk-test-cluster1-0-0) are not created. Only the keeper-test service appears, and the per-replica services disappear. This doesn't happen with clusterServiceTemplate, but the annotations still don't apply.

Thanks !!!

[carloss4dv]

Hi! I took a look at this issue and I think I might have found what's going on, though I'm not 100% sure. Let me share what I discovered while digging through the code.

What I Think Is Happening

From what I can see, there seems to be an architectural difference between how ClickHouseInstallation (CHI) and ClickHouseKeeperInstallation (CHK) handle service template resolution, and I believe this might be causing the problem.

What I Found in the Code

I was looking at the service creation logic in both pkg/model/chk/creator/service.go and pkg/model/chi/creator/service.go, and they both seem to follow the same pattern:

// Line 203 in both files
if template, ok := host.GetServiceTemplate(); ok {
    // Create service from template
}

But here's what I think might be the issue: the GetServiceTemplate() method appears to be shared between CHI and CHK, even though their data structures are quite different.

From what I understand:

CHI Structure:

• Clusters → Layout → Shards → Hosts
• Clusters → Layout → Replicas → Hosts

CHK Structure (based on pkg/apis/clickhouse-keeper.altinity.com/v1/):

• Clusters → Layout → Shards (containing Hosts []*apiChi.Host serialized as "replicas")
• Clusters → Layout → Replicas (containing Hosts []*apiChi.Host serialized as "shards")

The Potential Problem

Looking at the current GetServiceTemplate() implementation in pkg/apis/clickhouse.altinity.com/v1/type_host.go (around lines 252-258), it seems like it only checks the host's own templates:

func (host *Host) GetServiceTemplate() (*ServiceTemplate, bool) {
	if !host.GetTemplates().HasReplicaServiceTemplate() {
		return nil, false
	}
	name := host.GetTemplates().GetReplicaServiceTemplate()
	return host.GetCR().GetServiceTemplate(name)
}

My theory is that in CHK, when you specify replicaServiceTemplate at the replica level, the hosts that are nested within shards can't find it because they're only looking at their own templates, not at their parent replica's templates.

Possible Solution (Just an idea!)

If my understanding is correct, maybe we need a CHK-specific implementation of GetServiceTemplate() that searches through the hierarchy properly:

1. Check the host's own templates first
2. If not found, check the containing shard's templates
3. If still not found, check the containing replica's templates

Something like creating a pkg/apis/clickhouse-keeper.altinity.com/v1/type_host.go file with:

package v1

import (
	apiChi "github.com/altinity/clickhouse-operator/pkg/apis/clickhouse.altinity.com/v1"
)

// GetServiceTemplate gets service template for CHK hosts
// This searches through the hierarchy: host -> shard -> replica
func (host *apiChi.Host) GetServiceTemplate() (*apiChi.ServiceTemplate, bool) {
	// First, check if host has its own replicaServiceTemplate
	if host.GetTemplates() != nil && host.GetTemplates().HasReplicaServiceTemplate() {
		name := host.GetTemplates().GetReplicaServiceTemplate()
		return host.GetCR().GetServiceTemplate(name)
	}
	
	// If not found in host, check the shard that contains this host
	if shard := host.GetShard(); shard != nil {
		if chkShard, ok := shard.(*ChkShard); ok {
			if chkShard.GetTemplates() != nil && chkShard.GetTemplates().HasReplicaServiceTemplate() {
				name := chkShard.GetTemplates().GetReplicaServiceTemplate()
				return host.GetCR().GetServiceTemplate(name)
			}
		}
	}
	
	// If still not found, check the replica that contains the shard
	if cluster := host.GetCluster(); cluster != nil {
		if chkCluster, ok := cluster.(*Cluster); ok {
			// Find the replica that contains this host
			var containingReplica *ChkReplica
			chkCluster.WalkReplicas(func(index int, replica *ChkReplica) error {
				for _, replicaHost := range replica.Hosts {
					if replicaHost == host {
						containingReplica = replica
						return nil
					}
				}
				return nil
			})
			
			if containingReplica != nil && containingReplica.GetTemplates() != nil && containingReplica.GetTemplates().HasReplicaServiceTemplate() {
				name := containingReplica.GetTemplates().GetReplicaServiceTemplate()
				return host.GetCR().GetServiceTemplate(name)
			}
		}
	}
	
	return nil, false
}

Why This Might Work

If I'm on the right track, this approach could:

1. Keep everything backward compatible (host-level templates still work)
2. Allow proper template inheritance from shard and replica levels
3. Only affect CHK without breaking CHI functionality
4. Leverage Go's method resolution to use the CHK-specific implementation

What Should Happen

If this fix works, we should see:

• clusterServiceTemplate continues working as before
• replicaServiceTemplate actually creates the individual replica services (like chk-test-cluster1-0-0)
• Cilium annotations get applied properly to each service
• No more disappearing services when using replicaServiceTemplate

---

Disclaimer: This is just my first-pass analysis based on poking around the code for a bit. I might be completely wrong about this! 😅 I'd definitely recommend testing.

Thanks for developing this operator !!!