Add query ingress enable switch

[secat]

This PR addresses issue #30.

Update the jaeger query specification with an ingress specification containing an enable flag. When not set, defaults to creating the query ingress object resource.

[jpkrohling]

This change is 

[codecov]

Codecov Report

Merging #36 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #36      +/-   ##
==========================================
+ Coverage   99.16%   99.16%   +<.01%     
==========================================
  Files          16       16              
  Lines         599      601       +2     
==========================================
+ Hits          594      596       +2     
  Misses          5        5

Impacted Files	Coverage Δ
pkg/deployment/query.go	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 22e6a6a...79f9b0c. Read the comment docs.

[jpkrohling]

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @secat)

---

pkg/apis/io/v1alpha1/types.go, line 43 at r1 (raw file):

// JaegerQuerySpec defines the options to be used when deploying the query
type JaegerQuerySpec struct {
	Ingress JaegerQueryIngressSpec `json:"ingress"`

This struct could probably be named JaegerIngressSpec instead of JaegerQueryIngressSpec. If we ever need it for the collector or for a future new component, we could reuse it.

---

pkg/apis/io/v1alpha1/types.go, line 51 at r1 (raw file):

// JaegerQueryIngressSpec defines the options to be used when deploying the query ingress
type JaegerQueryIngressSpec struct {
	Enabled *bool `json:"enabled"`

Any reason to have it as a pointer?

---

pkg/deployment/query.go, line 120 at r1 (raw file):

// Ingresses returns a list of ingress rules to be deployed along with the all-in-one deployment
func (q *Query) Ingresses() []*v1beta1.Ingress {
	if q.jaeger.Spec.Query.Ingress.Enabled == nil || *q.jaeger.Spec.Query.Ingress.Enabled == true {

If Enabled were a bool instead of a *bool, we could avoid the null check :)

[secat]

---

pkg/apis/io/v1alpha1/types.go, line 43 at r1 (raw file):

Previously, jpkrohling (Juraci Paixão Kröhling) wrote…

This struct could probably be named JaegerIngressSpec instead of JaegerQueryIngressSpec. If we ever need it for the collector or for a future new component, we could reuse it.

Excellent idea, I will make the change.

[secat]

---

pkg/apis/io/v1alpha1/types.go, line 51 at r1 (raw file):

Previously, jpkrohling (Juraci Paixão Kröhling) wrote…

Any reason to have it as a pointer?

It is in order to not break the default behavior. Currently a user doesn't have to specify if the ingress is enabled, it is by default.

Also, using a pointer helps detecting that case, i.e. if the flag is set or not. A nil pointer means it was not set, therefore use the default value.

If we do not use a pointer, the default value will be false if the user doesn't set the flag value.

Which direction should we go?

[jpkrohling]

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jpkrohling and @secat)

---

pkg/apis/io/v1alpha1/types.go, line 51 at r1 (raw file):

the default value will be false if the user doesn't set the flag value

That's true. If we could set the default value to true, we wouldn't need a pointer, but you are right here. I'm fine with having it as a pointer then.

[secat]

---

pkg/apis/io/v1alpha1/types.go, line 43 at r1 (raw file):

Previously, secat (Serge Catudal) wrote…

Excellent idea, I will make the change.

I have pushed a commit which renames the struct from JaegerQueryIngressSpec to JaegerIngressSpec.

[jpkrohling]

Reviewed 3 of 3 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved

[jpkrohling]

Thanks!