Skip to content

Conversation

lizzy-0323
Copy link
Contributor

Summary

Now we can use okctl to connect to cluster by sys tenant, the function for connecting to other tenant is in progress.

Solution Description

@CLAassistant
Copy link

CLAassistant commented Jul 8, 2025

CLA assistant check
All committers have signed the CLA.

@lizzy-0323
Copy link
Contributor Author

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.1 out of 2 committers have signed the CLA.✅ lizzy-0323❌ liziyi17

liziyi17 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

done

@lizzy-0323
Copy link
Contributor Author

lizzy-0323 commented Jul 8, 2025

All of the issues found in golang-ci are type check, and most of them seems to be in previous code.

image

By the way, I thick this is because, for the combined type, golang cicannot find the corresponding field or method.

@lizzy-0323
Copy link
Contributor Author

@chris-sun-star Could you please see this?

if err != nil {
if kubeerrors.IsNotFound(err) {
return fmt.Errorf("OBCluster %s not found", o.Name)
} else {
Copy link
Member

Choose a reason for hiding this comment

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

Please fix the lint problem showed in this page.

Copy link
Member

@chris-sun-star chris-sun-star Jul 17, 2025

Choose a reason for hiding this comment

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

There's only 2 problems, you don't need to handle the previously pushed code.

return fmt.Errorf("password is not specified and not found in secrets")
}

if o.TenantName != DEFAULT_TENANT_NAME && o.User != DEFAULT_USER {
Copy link
Member

Choose a reason for hiding this comment

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

Is here something missing

logger := utils.GetDefaultLoggerInstance()
cmd := &cobra.Command{
Use: "connect <cluster_name>",
Short: "Connect to an ob cluster by sys tenant",
Copy link
Member

Choose a reason for hiding this comment

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

It's not only sys tenant

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So actually, we just need one single command for tenant module?

Copy link
Member

Choose a reason for hiding this comment

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

I just mean the description, you've provided an option to pass the tenant name

@chris-sun-star
Copy link
Member

@chris-sun-star Could you please see this?
OK, Sorry for the late reply.
Thanks for your contribution.

@lizzy-0323
Copy link
Contributor Author

lizzy-0323 commented Jul 18, 2025

@chris-sun-star Could you please see this?
OK, Sorry for the late reply.
Thanks for your contribution.

Thanks again for your comment~ And I will fix the problem then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants