Skip to content

Commit

Permalink
Copy stats types from upstream.
Browse files Browse the repository at this point in the history
This drops another dependency on k8s.io/kubernetes.
This does have the unfortunate side effect that implementers will now
get a compile error until they update their code to use the new type.

Just as a note:

The stats types have moved to k8s.io/kubelet, however the stats types
are only there as of v1.20.
Currently we support older versions than v1.20, and even our go.mod
imports from v1.19.

For now we copy the types in. Later we can remove the type defs and
change them to type aliases to the k8s.io/kubelet types (which prevents
another compile time issue).

Anything relying on type assertions to determine if something implements
this method will, unfortunately, be broken and it will be hard to notice
until runtime. We need to make sure to call this out in the release
notes.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
  • Loading branch information
cpuguy83 committed May 5, 2021
1 parent baa0e6e commit 8437e23
Show file tree
Hide file tree
Showing 7 changed files with 360 additions and 8 deletions.
2 changes: 1 addition & 1 deletion cmd/virtual-kubelet/internal/provider/mock/mock.go
Expand Up @@ -13,11 +13,11 @@ import (
"github.com/virtual-kubelet/virtual-kubelet/errdefs"
"github.com/virtual-kubelet/virtual-kubelet/log"
"github.com/virtual-kubelet/virtual-kubelet/node/api"
stats "github.com/virtual-kubelet/virtual-kubelet/node/api/statsv1alpha1"
"github.com/virtual-kubelet/virtual-kubelet/trace"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
stats "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1"
)

const (
Expand Down
4 changes: 2 additions & 2 deletions cmd/virtual-kubelet/internal/provider/provider.go
Expand Up @@ -6,8 +6,8 @@ import (

"github.com/virtual-kubelet/virtual-kubelet/node"
"github.com/virtual-kubelet/virtual-kubelet/node/api"
"github.com/virtual-kubelet/virtual-kubelet/node/api/statsv1alpha1"
v1 "k8s.io/api/core/v1"
stats "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1"
)

// Provider contains the methods required to implement a virtual-kubelet provider.
Expand All @@ -32,5 +32,5 @@ type Provider interface {

// PodMetricsProvider is an optional interface that providers can implement to expose pod stats
type PodMetricsProvider interface {
GetStatsSummary(context.Context) (*stats.Summary, error)
GetStatsSummary(context.Context) (*statsv1alpha1.Summary, error)
}
2 changes: 1 addition & 1 deletion internal/test/e2e/framework/stats.go
Expand Up @@ -5,8 +5,8 @@ import (
"encoding/json"
"strconv"

stats "github.com/virtual-kubelet/virtual-kubelet/node/api/statsv1alpha1"
"k8s.io/apimachinery/pkg/util/net"
stats "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1"
)

// GetStatsSummary queries the /stats/summary endpoint of the virtual-kubelet and returns the Summary object obtained as a response.
Expand Down
4 changes: 2 additions & 2 deletions node/api/stats.go
Expand Up @@ -20,11 +20,11 @@ import (
"net/http"

"github.com/pkg/errors"
stats "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1"
"github.com/virtual-kubelet/virtual-kubelet/node/api/statsv1alpha1"
)

// PodStatsSummaryHandlerFunc defines the handler for getting pod stats summaries
type PodStatsSummaryHandlerFunc func(context.Context) (*stats.Summary, error)
type PodStatsSummaryHandlerFunc func(context.Context) (*statsv1alpha1.Summary, error)

// HandlePodStatsSummary makes an HTTP handler for implementing the kubelet summary stats endpoint
func HandlePodStatsSummary(h PodStatsSummaryHandlerFunc) http.HandlerFunc {
Expand Down
7 changes: 7 additions & 0 deletions node/api/statsv1alpha1/README.md
@@ -0,0 +1,7 @@
These types are copied from the [k8s.io/kubelet](https://pkg.go.dev/k8s.io/kubelet@v0.21.0/pkg/apis/stats/v1alpha1) module.
They are used from a type alias in the API package.

It is being used this way because the module is only available from 1.20 and on, but currently we are pinned to v1.19 and plan to continue to support v1.19 for some time.
Likewise we want to stop importing k8s.io/kubernetes (where the older type def is) since this transatively imports all of kubernetes.

After the min version is v1.20 we can update the type alias to point the the module and remove these type definitions.

0 comments on commit 8437e23

Please sign in to comment.