Add ability to specify system namespace during cluster add operation #1661
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1661 +/- ##
==========================================
- Coverage 32.77% 32.53% -0.25%
==========================================
Files 71 71
Lines 11378 11029 -349
==========================================
- Hits 3729 3588 -141
+ Misses 7098 6919 -179
+ Partials 551 522 -29
Continue to review full report at Codecov.
|
|
Again, this passes both "test" and "test-e2e" on my workstation and Argo CI throws the following error at the E2E stage: time="2019-05-29T17:00:52Z" level=info msg="ran command" duration=80.687149ms err="exit status 1" |
|
Hi @alexec I have done both - merging from master and make pre-commit - already before submitting the PR. Actually, I branched off from latest upstream master, then pulled in the relevant changes from a previous local branch selectively. I'm still rather new to collaborating on GitHub (and also to working with git as a tool for that matter). What is the expected outcome of doing it again? Running make protogen also re-generated some of the other files, which got me quite confused - I didn't include them in the PR, because I neither touched their sources nor do I understand the changes yet: Shall I push them to the PR as well? Regarding listing my organization in the README - it's not that simple, unfortunately... I work with a fairly huge and very public corporation which has certain processes and regulations in place before having their name displayed in a public place. I already initiated a request for reviewing whether that's possible - because I'd love to see us in there, too :) Anyway, all my code contributions are created in my free time - I've got no coder job in the corp (read: no time for coding during work hours), but I'm working towards the broad adoption of ArgoCD for our K8S-based projects. |
|
Hmmmmmmm... do you want to write tests for this? |
|
Sure, let me have a look at how the tests work - I guess the right place for this would be in the end-to-end test suite? As far as I understood up until now, the end-to-end tests work without the ArgoCD components running in a k8s cluster but running local. I'll dig my way in, any pointers are appreciated! |
|
I think you should be able to write unit tests for this. I don't think E2E tests support different namespaces, but please prove me wrong! https://argoproj.github.io/argo-cd/developer-guide/test-e2e/ |
|
I seem to not get my head around this. Both, unit and e2e tests do not work with an installation of ArgoCD running in a K8S cluster. This change however affects integration of a k8s cluster with ArgoCD, and only at the time when we incorporate a remote k8s cluster into ArgoCD configuration for the first time. Not saying a test would not be important, but I cannot seem to find a valid test-case for this change in the current construct :-( |
|
If there’s no straight forward valid test, we can probably skip it. |
|
Note, I think you still need to sync with master. |
|
Shall I commit above mentioned protobuf generated files as well? |
|
hi - the |
|
I've merged so we can move forward. |
|
@alexec Thanks and sorry for the delay in my answer, I was away for some days. Checked the version of my protoc toolchain, guess I'm lagging a bit behind (3.6.1). Will upgrade for the next PRs, thank you for the hint. |
This change addresses #1595 by adding the --system-namespace option to the "cluster add" command of argocd CLI as well as a corresponding field in the API function:
[eris@docker-build argo-cd]$ ./dist/argocd cluster add --help | grep namespace
--system-namespace string Use different system namespace (default "kube-system")
When given, ArgoCD will create the System Account "argocd-manager" in the specified namespace. The change is non-breaking, defaults to previous behaviour by using "kube-system" as namespace when the option was not specified.