clickhouse-operator icon indicating copy to clipboard operation
clickhouse-operator copied to clipboard

About Helm and some question about chop

Open TCeason opened this issue 4 years ago ā€¢ 12 comments

@TCeason main parts of operator code contains in cmd and pkg directories, I you want to help you can try to help us with Helm chart feel free continue work with https://github.com/Altinity/clickhouse-operator/pull/358

Originally posted by @Slach in https://github.com/Altinity/clickhouse-operator/issues/619#issuecomment-747246135

This pr https://github.com/Altinity/clickhouse-operator/pull/358 is closed, so I will open a new issue to describe my question and just associated it so that will not annoy the committer.

TCeason avatar Dec 17 '20 09:12 TCeason

Yes, I saw some code in cmd/operator is the ch-op's entry function.

And some of the code in pkg dir is generated by code-generator.

And zz_generated.deepcopy.go is generated. But when the code was be generated? And I can not find the usage of dev/run_code_generator.sh

pkg šŸ‘† git:(master)  tree -L 1
.
ā”œā”€ā”€ apis   # some 
ā”œā”€ā”€ chop
ā”œā”€ā”€ client
ā”œā”€ā”€ controller
ā”œā”€ā”€ model
ā”œā”€ā”€ util
ā””ā”€ā”€ version

Now in my mind:

  • (1) build manifest via build-clickhouse-operator-install-yaml.sh

    a> In this step generate manifest clickhouse-operator-install.yaml

    b> the mainfest defines

    1. A crd named clickhouseinstallations.clickhouse.altinity.com
    2. A crd named clickhouseinstallationtemplates.clickhouse.altinity.com
    3. A crd named clickhouseoperatorconfigurations.clickhouse.altinity.com
    4. A clickHouse-operator ServiceAccount
    5. A ClusterRoleBinding named clickhouse-operator-kube-system binding the ServiceAccount
    6. A deployment named clickhouse-operator contains two containers altinity/clickhouse-operator:x.y.z and altinity/metrics-exporter:x.y.z
    7. Some ConfigMaps named under will be Injected into the deployment

    etc-clickhouse-operator-files: Mount on /etc/clickhouse-operator. **But I'm not sure what it used for. ** etc-clickhouse-operator-confd-files, etc-clickhouse-operator-configd-files: Mount on /etc/clickhouse-operator/conf.d and /etc/clickhouse-operator/config.d. In config.d I know that can record some clickhouse config about log, listen_host, query_log and so on. But what the conf.d used for? I guess it maybe seem like the /etc/clickhouse-client/conf.d ? etc-clickhouse-operator-templatesd-files: Mount on /etc/clickhouse-operator/templates.d I'm not sure what it used for. etc-clickhouse-operator-usersd-files: Mount on /etc/clickhouse-operator/users.d. It is the same as /etc/clickhouse-server/users.d

  • (2) create a namespace via kubectl create namespace.

  • (3) Install operator via kubectl apply -f clickhouse-operator-install.yaml a> in this step Install a Complete operator in k8s.

I'd like to try, but I'm just at the learning stage. Best wishes.

TCeason avatar Dec 17 '20 09:12 TCeason

https://github.com/Altinity/clickhouse-operator/issues/262

TCeason avatar Dec 17 '20 09:12 TCeason

In code

// isCHITemplateExt returns true in case specified file has proper extension for a CHI template config file
func (config *OperatorConfig) isCHITemplateExt(file string) bool {
	switch util.ExtToLower(file) {
	case ".yaml":
		return true
	case ".json":
		return true
	}
	return false
}

// Read CHI template files
config.CHITemplateFiles = util.ReadFilesIntoMap(config.CHITemplatesPath, config.isCHITemplateExt)

the config.CHITemplatesPath in yaml is chiTemplatesPath: templates.d and the file in templates.d is end with exaple

/etc/clickhouse-operator/templates.d # ls
001-templates.json.example             default-pod-template.yaml.example      default-storage-template.yaml.example  readme

So maybe the config.CHITemplateFiles is nil?

I find the code, the def of useTemplates is copy from chi.Spec.UseTemplates. And Is it defines at there? So how to use it?

        if len(chi.Spec.UseTemplates) > 0 {
		useTemplates = make([]chiv1.ChiUseTemplate, len(chi.Spec.UseTemplates))
		copy(useTemplates, chi.Spec.UseTemplates)

		// UseTemplates must contain reasonable data, thus has to be normalized
		n.normalizeUseTemplates(&useTemplates)
	}

TCeason avatar Dec 17 '20 09:12 TCeason

@Slach Hi, could you help me with this question? Iā€™m eager to receive your feedback.

In code

// isCHITemplateExt returns true in case specified file has proper extension for a CHI template config file
func (config *OperatorConfig) isCHITemplateExt(file string) bool {
	switch util.ExtToLower(file) {
	case ".yaml":
		return true
	case ".json":
		return true
	}
	return false
}
// Read CHI template files
config.CHITemplateFiles = util.ReadFilesIntoMap(config.CHITemplatesPath, config.isCHITemplateExt)

the config.CHITemplatesPath in yaml is chiTemplatesPath: templates.d and the file in templates.d is end with exaple

/etc/clickhouse-operator/templates.d # ls
001-templates.json.example             default-pod-template.yaml.example      default-storage-template.yaml.example  readme

So maybe the config.CHITemplateFiles is nil?

I find the code, the def of useTemplates is copy from chi.Spec.UseTemplates. And Is it defines at there? So how to use it?

        if len(chi.Spec.UseTemplates) > 0 {
		useTemplates = make([]chiv1.ChiUseTemplate, len(chi.Spec.UseTemplates))
		copy(useTemplates, chi.Spec.UseTemplates)

		// UseTemplates must contain reasonable data, thus has to be normalized
		n.normalizeUseTemplates(&useTemplates)
	}

TCeason avatar Dec 21 '20 03:12 TCeason

Hello @TCeason

dev/run_code_generator.sh

this script run manually if you change something in pkg/apis

build manifest via build-clickhouse-operator-install-yaml.sh

yes it's a good idea, I think we need to add a separate script deploy/helm/build-clickhouse-operator-helm.sh where we create separate templates/*yaml.tpl with go-templates placeholders

the main goal of clickhouse-operator Helm chart should just install and configure clickhouse-operator CRD and related deployment, service etc. and nothing else

I think we need to create a separate ClickHouse installation Helm chart (builded with dev/build-clickhouse-clusters-helm.sh which should require main helm chart, for manipulation of kind: ClickhouseInstallation and kind: ClickhouseInstallationTemplate custom resources

also, I think we can use https://github.com/mikefarah/yq to generate Helm chart templates/*.yaml.tpl from deploy/operator/*.yaml and put some go-templates

/etc/clickhouse-operator/conf.d and /etc/clickhouse-operator/config.d, /etc/clickhouse-operator/users.d/

these folders used for Clickhouse server configuration files users.d/ will merge with /etc/clickhouse-server/users.xml config.d/ will merge with /etc/clickhouse-server/config.xml conf.d/ will merge with both, and doesn't use now

look to https://github.com/ClickHouse/ClickHouse/blob/master/programs/server/Server.cpp#L356 https://github.com/ClickHouse/ClickHouse/blob/40aa97d1cf1e97ba3e9860394c5604676d99ee8e/docs/en/operations/configuration_files.md

/etc/clickhouse-operator/templates.d

this folder use for store kind: ClickHouseInstallationTemplate yaml / json files which will apply directly to a kubernetes API server after each clickhouse-operator process start currently, this folder contains only examples and apply nothing to kubeapi URL

yes, we can use .ClickHouseInstallationTemplate.metadata.name from teamplates.d files in useTemplates section in kind: ClickHouseInstallation custom resource instances

Slach avatar Dec 22 '20 07:12 Slach

Dear Klimov, thanks for your suggestion . I will have a look at yq.

TCeason avatar Dec 22 '20 13:12 TCeason

helm lint 
==> Linting .
[INFO] Chart.yaml: icon is recommended
[ERROR] templates/clickhouse-operator-install-crd.yaml: the kind "apiextensions.k8s.io/v1beta1 CustomResourceDefinition" is deprecated in favor of "apiextensions.k8s.io/v1 CustomResourceDefinition"

Is there have some problems that we can not support k8s 1.16+?

The default field can be set when the Defaulting feature is enabled, which is the case with apiextensions.k8s.io/v1 CustomResourceDefinitions. 

Defaulting is in GA since 1.17 (beta since 1.16 with the CustomResourceDefaulting feature gate enabled, which is the case automatically for many clusters for beta features).

https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/

TCeason avatar Dec 23 '20 07:12 TCeason

And In file build-clickhouse-operator-install-yaml.sh, build part templated manifests twice.

https://github.com/Altinity/clickhouse-operator/blob/master/deploy/operator/build-clickhouse-operator-install-yaml.sh#L50

https://github.com/Altinity/clickhouse-operator/blob/master/deploy/operator/build-clickhouse-operator-install-yaml.sh#L98

And not generate a rbac.yaml just generate a template-rbac.yaml.

Is there has some other sense?

Now I want to use this arch:

.
ā”œā”€ā”€ Chart.yaml
ā”œā”€ā”€ templates
ā”‚   ā””ā”€ā”€ clickhouse-operator-install.yaml
ā””ā”€ā”€ values.yaml

And templates/clickhouse-operator-install.yaml will be generated from deploy/operator/clickhouse-operator-install.yaml.

What do you think about it? @Slach

TCeason avatar Dec 23 '20 07:12 TCeason

I agree with your file layout and think your file layour should be present in deploy/helm/operator and the second chart should present in 'deploy/helm/cluster-management`

@sunsingerus could you explain to us differences between https://github.com/Altinity/clickhouse-operator/blob/master/deploy/operator/build-clickhouse-operator-install-yaml.sh#L50 and https://github.com/Altinity/clickhouse-operator/blob/master/deploy/operator/build-clickhouse-operator-install-yaml.sh#L98?

from my side it looks as copy\pasted code ;) but little differences look like files generated twice, first time without namespace, and with templated namespace as $OPERATOR_NAMESPACE at the second time

Slach avatar Dec 23 '20 08:12 Slach

helm lint 
==> Linting .
[INFO] Chart.yaml: icon is recommended
[ERROR] templates/clickhouse-operator-install-crd.yaml: the kind "apiextensions.k8s.io/v1beta1 CustomResourceDefinition" is deprecated in favor of "apiextensions.k8s.io/v1 CustomResourceDefinition"

Is there have some problems that we can not support k8s 1.16+?

The default field can be set when the Defaulting feature is enabled, which is the case with apiextensions.k8s.io/v1 CustomResourceDefinitions. 

Defaulting is in GA since 1.17 (beta since 1.16 with the CustomResourceDefaulting feature gate enabled, which is the case automatically for many clusters for beta features).

https://kubernetes.io/docs/tasks/extend-kubernetes/custom-resources/custom-resource-definitions/

will chop support apiextensions.k8s.io/v1?

TCeason avatar Dec 23 '20 09:12 TCeason

will chop support apiextensions.k8s.io/v1?

Do you have any issues with v1beta1? If we switch CRD to v1, that will mean dropping compatibility with k8s 1.15 and earlier.

alex-zaitsev avatar Dec 23 '20 12:12 alex-zaitsev

will chop support apiextensions.k8s.io/v1?

Do you have any issues with v1beta1? If we switch CRD to v1, that will mean dropping compatibility with k8s 1.15 and earlier.

Yes. Now I use helm 3.0 runs helm lint and then it returns an error.

[ERROR] templates/clickhouse-operator-install-crd.yaml: the kind "apiextensions.k8s.io/v1beta1 CustomResourceDefinition" is deprecated in favor of "apiextensions.k8s.io/v1 CustomResourceDefinition"

I think there are some compatibility issues.

TCeason avatar Dec 23 '20 13:12 TCeason