-
Notifications
You must be signed in to change notification settings - Fork 15
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
Closes #140 - Upgrading to kubebuilder format and SDK 1.0.1 #175
Conversation
@LCaparelli There's a lot of clutter in this PR. It's not ready yet. I forgot to change to draft, don't even waste your time reviewing it 😅😅 |
Signed-off-by: Ricardo Zanini <[email protected]>
@LCaparelli @Kaitou786 @Kevin-Mok we're ready, I believe. If you guys have the time, please could you take a look? I'll get back at the end of the day to apply your suggestions, if any. There's a lot to do, but I believe I covered the vast majority in this PR. We can keep improving. Since 0.4.0 will be released at the end of the month, I think we will have the time to find any bugs that might come up duo to the migration. |
e747a84
to
13e2839
Compare
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.
They changed a lot but this is nice <3
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.
Thanks a lot for this @ricardozanini, looks like it was a handful. Also sorry for the delay.
Left some comments in
Signed-off-by: Ricardo Zanini <[email protected]>
Signed-off-by: Ricardo Zanini <[email protected]>
@LCaparelli I believe I've covered all your comments, can you take a look again please? |
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.
Thanks again @ricardozanini :-)
@@ -87,32 +87,32 @@ func (v *Validator) validateNetworking(nexus *v1alpha1.Nexus) error { | |||
} | |||
|
|||
if !v.ingressAvailable && nexus.Spec.Networking.ExposeAs == v1alpha1.IngressExposeType { | |||
v.log.Info("Ingresses are not available on your cluster. Make sure to be running Kubernetes > 1.14 or if you're running Openshift set ", "spec.networking.exposeAs", v1alpha1.IngressExposeType, "Also try", v1alpha1.NodePortExposeType) | |||
v.log.Warn("Ingresses are not available on your cluster. Make sure to be running Kubernetes > 1.14 or if you're running Openshift set ", "spec.networking.exposeAs", v1alpha1.IngressExposeType, "Also try", v1alpha1.NodePortExposeType) |
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.
So you went with warnings, huh? Up to you, but I still think we should log it as error. Shouldn't cause too much trouble after #174 anyways
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.
Take a look in the next line, you will see that there's an error being raised.
Closes #140
TODO:
😓
Signed-off-by: Ricardo Zanini [email protected]