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

feat: add helm for k8s and makefile #444

Merged
merged 1 commit into from
Feb 15, 2022
Merged

Conversation

henrywangx
Copy link
Contributor

No description provided.

@casbin-bot
Copy link
Contributor

@seriouszyx @tangyang9464 please review

@CLAassistant
Copy link

CLAassistant commented Jan 24, 2022

CLA assistant check
All committers have signed the CLA.

@henrywangx henrywangx force-pushed the xiowang branch 2 times, most recently from a99710f to b3d3194 Compare January 24, 2022 05:47
@henrywangx henrywangx changed the title add helm for k8s and makefile feat: add helm for k8s and makefile Jan 24, 2022
@henrywangx henrywangx force-pushed the xiowang branch 6 times, most recently from fa98ca0 to f8d1504 Compare January 24, 2022 11:33
@hsluoyz
Copy link
Member

hsluoyz commented Jan 24, 2022

@henrywangx plz fix:

image

@henrywangx
Copy link
Contributor Author

henrywangx commented Jan 24, 2022

Contributor License Agreement

@hsluoyz, I have signed with my account. Suppose it's ok now

@hsluoyz
Copy link
Member

hsluoyz commented Jan 25, 2022

@ComradeProgrammer @Steve0x2a @seriouszyx plz review

@henrywangx henrywangx force-pushed the xiowang branch 4 times, most recently from 538f7b0 to 0a3e70e Compare January 25, 2022 03:13
main.go Outdated Show resolved Hide resolved
@ComradeProgrammer
Copy link
Contributor

ComradeProgrammer commented Jan 25, 2022

Well I think I have some questions:

  1. the port problems mentioned
  2. do we really need to introduce makefile?
  3. do we really need to introduce go vendor?
  4. I think you may have some misunderstandings about the casdoor images (correct me if I am wrong plz). In the makefile I realize that you are trying to deploy the all-in-one image into k8s, which I think is improper. This image is supposed to be used for new users to experience casdoor with simple commands. This image integrates a database and have some special settings for the simplicication of deploying, thus inappropriate to be deployed into productive environments, like k8s
  5. considering that it is very likely for potential users of casdoor to maintain their own customized image instead of our image to meet their own demand, I wonder whether it's proper and necessary to publish (or introduce) casdoor via helm, despite the truth that helm is very influential in cloud native area. In my own opinion, a more appropriate way may be giving out some simple config files to demostrate how to deploy it into k8s and what is needed to be noticed(for example, to mount the conf folder..) and it's protential customers' own duty and right to choose how to deploy it into k8s according to the various customized demands. A user who have reached the step of deploying casdoor into k8s should have their own deploying methods and convention, which is very unlikely to be identical to the configurations we write in helm charts.

@hsluoyz
Copy link
Member

hsluoyz commented Jan 25, 2022

@henrywangx can you join the QQ group: 645200447 for discussion?

@henrywangx
Copy link
Contributor Author

Well I think I have some questions:

  1. the port problems mentioned
  2. do we really need to introduce makefile?
  3. do we really need to introduce go vendor?
  4. I think you may have some misunderstandings about the casdoor images (correct me if I am wrong plz). In the makefile I realize that you are trying to deploy the all-in-one image into k8s, which I think is improper. This image is supposed to be used for new users to experience casdoor with simple commands. This image integrates a database and have some special settings for the simplicication of deploying, thus inappropriate to be deployed into productive environments, like k8s
  5. considering that it is very likely for potential users of casdoor to maintain their own customized image instead of our image to meet their own demand, I wonder whether it's proper and necessary to publish (or introduce) casdoor via helm, despite the truth that helm is very influential in cloud native area. In my own opinion, a more appropriate way may be giving out some simple config files to demostrate how to deploy it into k8s and what is needed to be noticed(for example, to mount the conf folder..) and it's protential customers' own duty and right to choose how to deploy it into k8s according to the various customized demands. A user who have reached the step of deploying casdoor into k8s should have their own deploying methods and convention, which is very unlikely to be identical to the configurations we write in helm charts.
  1. I will change the default port.
  2. Yes, we should. For a production code, we need to define the build, containerized, deploy, ut, lint and some other necessary functions.
  3. In my MR, I just remove it from .gitignore. That means I won't upload vendor.
  4. It's default name for container. I will change the default name. And you could pass the env, if you need to customize the name. Both of registry and container name can be changed.
  5. If some user don't like helm, they could add other commands into makefile(That's why I introduce the makefile). But If we don't support helm, we will lost many users.

Signed-off-by: henrywangx <henrywangx@gmail.com>
@hsluoyz
Copy link
Member

hsluoyz commented Feb 3, 2022

@henrywangx I'm OK to have better integration with k8s and helm, but this PR seems to add too many things that don't natively belong to Casdoor. Can we put this PR into some places like a casdoor-k8s-plugin? In the plugin repo, we can refer to Casdoor repo (this repo) via git submodules or other methods. What do you think?

@henrywangx
Copy link
Contributor Author

henrywangx commented Feb 10, 2022

@hsluoyz
This PR contains bellow things:

  1. how to containerized casdoor;
  2. enhance casdoor's code with golang-lint;
  3. add helm chart for k8s;

All those things should be necessary, since I found it's too difficult to deploy casdoor into production env. If community think it's too heavy, just close this PR.

@hsluoyz hsluoyz merged commit e35b058 into casdoor:master Feb 15, 2022
@casbin-bot
Copy link
Contributor

🎉 This PR is included in version 1.17.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@hsluoyz
Copy link
Member

hsluoyz commented Feb 15, 2022

@henrywangx thanks for your contribution! Can you also add docs to: https://casdoor.org/docs/deploy/k8s ?

@henrywangx henrywangx deleted the xiowang branch February 16, 2022 03:37
Steve0x2a pushed a commit to Steve0x2a/casdoor that referenced this pull request Feb 16, 2022
Signed-off-by: henrywangx <henrywangx@gmail.com>

Co-authored-by: xiong wang <xiong.wang@inceptio.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants