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.

Sign up for GitHub

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

Jump to bottom

Support multi secret #242

Merged
merged 8 commits into from
Nov 27, 2024
Merged

Support multi secret #242

merged 8 commits into from
Nov 27, 2024

Conversation

hechao-ustc
Copy link
Contributor

issue: #237
Support multi secret:

spec:
  feSpec:
    secrets:
    # use kubectl create secret generic db-user --from-file=./username.txt -n doris
    - secretName: db-user
      mountPath: /etc/doris
      readOnly: true
    # use kubectl create secret generic db-pass --from-file=./password.txt -n doris
    - secretName: db-pass
      mountPath: /opt/doris
      readOnly: true

catpineapple
catpineapple reviewed Sep 11, 2024
View reviewed changes
pkg/common/utils/k8s/client.go Outdated
@@ -229,6 +229,20 @@ func GetConfigMap(ctx context.Context, k8scient client.Client, namespace, name s
return &configMap, nil
}

// CheckSecretExist check if the secret is in the namespace.
func CheckSecretExist(ctx context.Context, k8scient client.Client, namespace string, secrets []dorisv1.Secret) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This func is not necessary. After checking, it only prints the log and has no effect on subsequent operations. Checking whether it exists can be done later.

Copy link
Contributor Author

@hechao-ustc hechao-ustc Sep 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved CheckSecretExist to after k8s.ApplyStatefulSet. I think this check is necessary. Without this check and without created the secret, the operator log will not contain the following error log, and the pod will always be in the ContainerCreating state.
image

catpineapple
catpineapple reviewed Sep 13, 2024
View reviewed changes
api/doris/v1/types.go Outdated Show resolved Hide resolved
hechao added 4 commits September 18, 2024 14:21
Merge remote-tracking branch 'refs/remotes/upstream/master' into mult…
eb1567b 
…i_secret_0910

# Conflicts:
#	config/crd/bases/crds.yaml
#	config/crd/bases/disaggregated.cluster.doris.com_dorisdisaggregatedclusters.yaml
#	pkg/controller/sub_controller/disaggregated_cluster/computeclusters/controller.go
multi secret
f8c4cba
multi secret
8ffbf55
Merge remote-tracking branch 'refs/remotes/upstream/master' into mult…
2ce8067 
…i_secret_0910
catpineapple
catpineapple approved these changes Sep 23, 2024
View reviewed changes
Copy link
Contributor

@catpineapple catpineapple left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

intelligentfu8
intelligentfu8 reviewed Sep 23, 2024
View reviewed changes
api/doris/v1/types.go

// Current Secret Mount Path, default is "/etc/doris"
// If Secret belongs to the same Secrets, their MountPath cannot be repeated.
// +optional
Copy link
Contributor

@intelligentfu8 intelligentfu8 Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mountPath can't be repeated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

intelligentfu8
intelligentfu8 reviewed Sep 23, 2024
View reviewed changes
doc/examples/doriscluster-sample-secret.yaml

# this yaml describe `secret` config in DorisCluster CRD.
#Compareed to `doriscluster-sample.yaml`, It have `secret` config for fe
apiVersion: doris.selectdb.com/v1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add secrets that used in the example, in front of the yaml.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

intelligentfu8
intelligentfu8 reviewed Sep 23, 2024
View reviewed changes
pkg/common/utils/k8s/client.go Outdated
@@ -229,6 +229,20 @@ func GetConfigMap(ctx context.Context, k8scient client.Client, namespace, name s
return &configMap, nil
}

// CheckSecretExist check if the secret is in the namespace.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ChecSecretExist, check the secret exist or not in specify namespace.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, and I moved this method to sub_controller.go

intelligentfu8
intelligentfu8 reviewed Sep 23, 2024
View reviewed changes
pkg/common/utils/resource/pod.go Outdated
defaultMountPath := ""
switch componentType {
case v1.Component_FE, v1.Component_BE, v1.Component_CN, v1.Component_Broker:
defaultMountPath = config_env_path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't use env_config_path,should use secret_config_path

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

intelligentfu8
intelligentfu8 reviewed Sep 23, 2024
View reviewed changes
pkg/common/utils/resource/pod.go Outdated
var volumes []corev1.Volume
var volumeMounts []corev1.VolumeMount

if bSpec.Secrets == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check is not needed. for golang splice, iterate the nil splice is ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

intelligentfu8
intelligentfu8 reviewed Sep 23, 2024
View reviewed changes
pkg/controller/sub_controller/be/controller.go Outdated
@@ -99,6 +100,11 @@ func (be *Controller) Sync(ctx context.Context, dcr *v1.DorisCluster) error {
return err
}

// check if the secret exists in the namespace
if dcr.Spec.BeSpec.Secrets != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this check should before "applystatefulset" and should output an event on dorislcuster.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

hechao added 2 commits October 29, 2024 17:14
Merge remote-tracking branch 'refs/remotes/upstream/master' into mult…
f4cb998 
…i_secret_0910

# Conflicts:
#	pkg/common/utils/resource/pod.go
#	pkg/controller/sub_controller/events.go
Merge remote-tracking branch 'refs/remotes/upstream/master' into mult…
cb87526 
…i_secret_0910
@intelligentfu8 intelligentfu8 merged commit c29e991 into apache:master Nov 27, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@intelligentfu8 intelligentfu8 intelligentfu8 approved these changes

@catpineapple catpineapple catpineapple approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@hechao-ustc @catpineapple @intelligentfu8