-
Notifications
You must be signed in to change notification settings - Fork 272
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: operator disables Ingress when spec.(component).host is empty #5305
Conversation
@andreaTP review please |
hi @andreaTP I've added two more commits with a fix and a workaround for remote tests. As I mentioned in our discussion, the remote tests do not work properly:
I can either bring up my suggested changes to the testsuite in a separate PR, or I can create an issue for you to fix the remote tests. |
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.
LGTM, thanks for keeping the scope limited to the change, appreciated!
Is the bug in the testing blocking you?
I would check what we are doing differently from KC.
No strong preference, I can look into the bug later on next week or review a separate PR with the fix, as you prefer 👍
operator/controller/src/test/java/io/apicurio/registry/operator/it/SmokeITTest.java
Outdated
Show resolved
Hide resolved
operator/controller/src/test/java/io/apicurio/registry/operator/it/SmokeITTest.java
Outdated
Show resolved
Hide resolved
It's not blocking, but it'll make tests harder to write. I'll recreate a PR with my changes and we can discuss them. I'll also take a look at KC, but I think we should not try too strongly to reproduce everything they do, if something works for us. |
…d due to bad generated resources
…es are deployed at once
No description provided.