-
Notifications
You must be signed in to change notification settings - Fork 81
[kiali] use kiali operator to install and manage kiali #622
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please run
BUILD_WITH_CONTAINER=0 make update-goldens
make clean gen
to make the tests pass.
In the CONTRIBUTING.adoc (or somewhere here - README maybe) you should explain to new developers what their GO env var should be. GOPATH / GOROOT. Mine are set for my own projects, but if I am to run
What should be my GOPATH/GOROOT? |
78d2a80
to
0bc837d
Compare
I think you need BUILD_WITH_CONTAINER=0 when running
At least when I do that, make is doing something. I dunno.. all I know is, its starting to work now. |
I have no idea what mandiff is telling me nor what is wrong. |
I ran "make update-charts" and "make generate-types generate-values" just to see if there is anything else I need to do (I'm just guessing - looking at the Makefiles to see what targets are available to run). They changed nothing. So, without documentation directing me on how to develop in this operator repo, I'm going to have to ask that someone "in the know" either tell me what else needs to be done or try to complete the changes required. Right now I'm stumped. |
@morvencao why must BUILD_WITH_CONTAINER=0 need be set? If the concern is over HUB/TAG overrides, a) I believe @howardjohn has fixed that b) if he hasn't I will fix it as soon as I have a test case where it is broken c) If there is some other concern, please let me know. This repo is on the BUILD_WITH_CONTAINER plan, and as such, there should be no good reason to turn off BUILD_WITH_CONTAINER - unless as a developer you want to develop with your own dependencies. Cheers |
@jmazzitelli my environment on Linux is:
TLDR;
That - coupled with other comments in this review, should get you going. Please have a read of this (very dated) wiki that explains how to install the various toolchains: |
I don't know either and it is very frustrating. When @ostromart says why - and you understand it, would you be kind enough to document it in our developer wiki: |
@jmazzitelli all of Istio's repos have a Cheers |
@sdake I assume you meant to include a link here? I just ran "make update-goldens" and got this:
There is something wrong in my branch to cause the failure, but I don't know what the error is (nothing in those messages above help)... if I run this in master branch, it finishes successfully. I tried with and without "export BUILD_WITH_CONTAINER=0" - both fail in my branch, but with no error messages to point me in the direction of how to fix. |
7f25802
to
d6e35ba
Compare
The error is because the "manifest generate" command fails with a exit code of 255. It is probably because I didn't update-charts. But I can't update charts because my other PR in istio/installer isn't merged yet, but of course I can't merge it until I can confirm THAT PR works, and I can only confirm THAT PR works if I can run and test THIS PR to confirm that it works. It's a catch-22. I can't merge the istio/installer PR until I confirm this istio/operator PR works, but I can't build/test this istio/operator PR until I merge istio/installer PR. So, in short, the way these two installer/operator repos work together make it pretty much impossible for a contributor to contribute to them. This needs to be fixed. As it is now, working in these two repos requires the person to have intimate knowledge of the make targets and the build scripts in both repos (none of this is documented) to get this to build and test locally to confirm everything works and get the tests to pass. |
@jmazzitelli I, and I think everyone else working on this, completely agrees its a huge mess right now. We are working right now to make this a lot simpler hopefully, see istio/istio#19146. Sorry that its in the current state right now |
d6e35ba
to
f3441e9
Compare
@jmazzitelli: The following tests failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
🚧 This issue or pull request has been closed due to not having had activity from an Istio team member since 2019-11-27. If you feel this issue or pull request deserves attention, please reopen the issue. Please see this wiki page for more information. Thank you for your contributions. Created by the issue and PR lifecycle manager. |
(work on this PR is on hold until the monorepo PR is merged, then we can merge the work done here and istio/installer#556)