Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

About Helm and some question about chop #620

Open
TCeason opened this issue Dec 17, 2020 · 12 comments
Open

About Helm and some question about chop #620

TCeason opened this issue Dec 17, 2020 · 12 comments
Labels
hold This issue has been put on hold

Comments

@TCeason
Copy link
Contributor

TCeason commented Dec 17, 2020

@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 #358

Originally posted by @Slach in #619 (comment)

This pr #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 TCeason changed the title @TCeason main parts of operator code contains in cmd and pkg directories, About Helm and some question about chop Dec 17, 2020
@TCeason
Copy link
Contributor Author

TCeason commented Dec 17, 2020

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
Copy link
Contributor Author

TCeason commented Dec 17, 2020

#262

@TCeason
Copy link
Contributor Author

TCeason commented Dec 17, 2020

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
Copy link
Contributor Author

TCeason commented Dec 21, 2020

@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)
	}

@Slach
Copy link
Collaborator

Slach commented Dec 22, 2020

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

@TCeason
Copy link
Contributor Author

TCeason commented Dec 22, 2020

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

@TCeason
Copy link
Contributor Author

TCeason commented Dec 23, 2020

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
Copy link
Contributor Author

TCeason commented Dec 23, 2020

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

@Slach
Copy link
Collaborator

Slach commented Dec 23, 2020

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

@TCeason
Copy link
Contributor Author

TCeason commented Dec 23, 2020

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?

@alex-zaitsev
Copy link
Member

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.

@TCeason
Copy link
Contributor Author

TCeason commented Dec 23, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hold This issue has been put on hold
Projects
None yet
Development

No branches or pull requests

4 participants