Skip to content

Commit

Permalink
Add tests
Browse files Browse the repository at this point in the history
Signed-off-by: Sivanantham Chinnaiyan <[email protected]>
  • Loading branch information
sivanantha321 committed Mar 7, 2024
1 parent 15fab58 commit 3adf60a
Show file tree
Hide file tree
Showing 5 changed files with 698 additions and 26 deletions.
7 changes: 4 additions & 3 deletions pkg/controller/v1beta1/inferenceservice/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ import (

"github.com/google/go-cmp/cmp"
"github.com/google/go-cmp/cmp/cmpopts"
"github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
"github.com/kserve/kserve/pkg/apis/serving/v1beta1"
"github.com/kserve/kserve/pkg/constants"
. "github.com/onsi/ginkgo/v2"
"github.com/onsi/gomega"
. "github.com/onsi/gomega"
Expand All @@ -46,6 +43,10 @@ import (
knservingv1 "knative.dev/serving/pkg/apis/serving/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/reconcile"

"github.com/kserve/kserve/pkg/apis/serving/v1alpha1"
"github.com/kserve/kserve/pkg/apis/serving/v1beta1"
"github.com/kserve/kserve/pkg/constants"
)

var _ = Describe("v1beta1 inference service controller", func() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,20 @@ import (
var (
log = logf.Log.WithName("IngressReconciler")
// probeTimeout defines the maximum amount of time a request will wait
probeTimeout = 1 * time.Second
probeTimeout = 1 * time.Second
Transport http.RoundTripper = &http.Transport{
Proxy: http.ProxyFromEnvironment,
TLSClientConfig: &tls.Config{
// #nosec G402
// We only want to know that the Ingress is configured, not that the configuration is valid.
// Therefore, we can safely ignore any TLS certificate validation.
InsecureSkipVerify: true,
},
TLSHandshakeTimeout: 2 * time.Second,
DisableKeepAlives: true,
IdleConnTimeout: 1 * time.Second,
ResponseHeaderTimeout: 1 * time.Second,
}
)

type IngressReconciler struct {
Expand Down Expand Up @@ -466,17 +479,9 @@ func probeIngress(url string) (bool, error) {
isReady := false
// Probes Queue-Proxy or Activator
target := url + knnethttp.HealthCheckPath
transport := http.DefaultTransport.(*http.Transport).Clone()
transport.TLSClientConfig = &tls.Config{
// #nosec G402
// We only want to know that the Ingress is configured, not that the configuration is valid.
// Therefore, we can safely ignore any TLS certificate validation.
InsecureSkipVerify: true,
}
transport.DisableKeepAlives = true
ctx, cancel := context.WithTimeout(context.TODO(), probeTimeout)
defer cancel()
req, err := http.NewRequest(http.MethodGet, target, nil)
req, err := http.NewRequestWithContext(ctx, http.MethodGet, target, nil)
if err != nil {
return isReady, errors.Wrapf(err, "failed to probe ingress %s", target)
}
Expand All @@ -486,19 +491,17 @@ func probeIngress(url string) (bool, error) {
req.Header.Add(knheader.HashKey, knheader.HashValueOverride)
// IngressReadinessUserAgent is the user-agent header value set in probe requests for Ingress status.
req.Header.Add(knheader.UserAgentKey, knheader.IngressReadinessUserAgent)
req = req.WithContext(ctx)
resp, err := transport.RoundTrip(req)
resp, err := Transport.RoundTrip(req)
if err != nil {
return isReady, errors.Wrapf(err, "failed to probe ingress %s", target)
}
log.Info("ingress probe result", "statuscode", resp.StatusCode)
if resp.StatusCode == http.StatusOK {
isReady = true
}
return isReady, nil
}

func (ir *IngressReconciler) isIngressReady(isvc *v1beta1.InferenceService) bool {
func isIngressReady(isvc *v1beta1.InferenceService) bool {
return isvc.Generation == isvc.Status.ObservedGeneration && isvc.Status.GetCondition(v1beta1.IngressReady).IsTrue()
}

Expand Down Expand Up @@ -578,7 +581,7 @@ func (ir *IngressReconciler) Reconcile(isvc *v1beta1.InferenceService) (ctrl.Res
Scheme: scheme,
},
}
if ir.isIngressReady(isvc) {
if isIngressReady(isvc) {
// When the ingress has already been marked Ready for this generation,
// then it must have been successfully probed. This exception necessary for the case
// of global resyncs.
Expand Down
Loading

0 comments on commit 3adf60a

Please sign in to comment.