Skip to content
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

Remove dependency on kubectl binary #181

Merged
merged 1 commit into from
Jun 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions builder/Dockerfile.linux
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ RUN go build ./cmd/aks-periscope
# Runner
FROM alpine

RUN apk --no-cache add ca-certificates curl openssl bash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 we checked and this seems good, we dint find any code with curl dependency et. al. hence removal of this seems justifiable, if we find something we can always add it back again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, just as note, the default of alpine shell is ash


ADD https://storage.googleapis.com/kubernetes-release/release/v1.16.0/bin/linux/amd64/kubectl /usr/local/bin/kubectl
RUN chmod +x /usr/local/bin/kubectl

COPY --from=builder /build/aks-periscope /

ENTRYPOINT ["/aks-periscope"]
1 change: 0 additions & 1 deletion docs/windows-vs-linux.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ The following collectors are currently unavailable on Windows:
- DNS: This relies on `resolv.conf`, which is unavailable in Windows.
- IPTables: The `iptables` command is not available on Windows.
- Kubelet: This shows the arguments used to invoke the kubelet process. Windows containers do not support shared process namespaces, and so we cannot see processes on the host node.
- OSM and SMI: These collectors currently invoke the `kubectl` binary as a shell command, rather than using the cross-platform `client-go` SDK.
- SystemLogs: This uses `journalctl` to retrieve system logs, which is not available on Windows.

## Node Logs differences
Expand Down
6 changes: 0 additions & 6 deletions pkg/collector/osm_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,6 @@ func (collector *OsmCollector) GetName() string {
}

func (collector *OsmCollector) CheckSupported() error {
// This is not currently supported on Windows because it launches `kubectl` as a separate process (within GetResourceList).
// If/when it is reimplemented using the go client API for k8s, we can re-enable this.
if collector.runtimeInfo.OSIdentifier != "linux" {
return fmt.Errorf("unsupported OS: %s", collector.runtimeInfo.OSIdentifier)
}

if !utils.Contains(collector.runtimeInfo.CollectorList, "OSM") {
return fmt.Errorf("not included because 'OSM' not in COLLECTOR_LIST variable. Included values: %s", strings.Join(collector.runtimeInfo.CollectorList, " "))
}
Expand Down
29 changes: 9 additions & 20 deletions pkg/collector/osm_collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,34 +26,24 @@ func TestOsmCollectorGetName(t *testing.T) {

func TestOsmCollectorCheckSupported(t *testing.T) {
tests := []struct {
name string
osIdentifier string
collectors []string
wantErr bool
name string
collectors []string
wantErr bool
}{
{
name: "windows",
osIdentifier: "windows",
collectors: []string{"OSM"},
wantErr: true,
name: "OSM not included",
collectors: []string{"NOT_OSM"},
wantErr: true,
},
{
name: "linux without OSM included",
osIdentifier: "linux",
collectors: []string{"NOT_OSM"},
wantErr: true,
},
{
name: "linux with OSM included",
osIdentifier: "linux",
collectors: []string{"OSM"},
wantErr: false,
name: "OSM included",
collectors: []string{"OSM"},
wantErr: false,
},
}

for _, tt := range tests {
runtimeInfo := &utils.RuntimeInfo{
OSIdentifier: tt.osIdentifier,
CollectorList: tt.collectors,
}
c := NewOsmCollector(nil, runtimeInfo)
Expand Down Expand Up @@ -107,7 +97,6 @@ func TestOsmCollectorCollect(t *testing.T) {
}

runtimeInfo := &utils.RuntimeInfo{
OSIdentifier: "linux",
CollectorList: []string{"OSM"},
}

Expand Down
6 changes: 0 additions & 6 deletions pkg/collector/smi_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,6 @@ func (collector *SmiCollector) GetName() string {
}

func (collector *SmiCollector) CheckSupported() error {
// This is not currently supported on Windows because it launches `kubectl` as a separate process (within GetResourceList).
// If/when it is reimplemented using the go client API for k8s, we can re-enable this.
if collector.runtimeInfo.OSIdentifier != "linux" {
return fmt.Errorf("unsupported OS: %s", collector.runtimeInfo.OSIdentifier)
}

if !utils.Contains(collector.runtimeInfo.CollectorList, "OSM") && !utils.Contains(collector.runtimeInfo.CollectorList, "SMI") {
return fmt.Errorf("not included because neither 'OSM' or 'SMI' are in COLLECTOR_LIST variable. Included values: %s", strings.Join(collector.runtimeInfo.CollectorList, " "))
}
Expand Down
36 changes: 12 additions & 24 deletions pkg/collector/smi_collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,40 +23,29 @@ func TestSmiCollectorGetName(t *testing.T) {

func TestSmiCollectorCheckSupported(t *testing.T) {
tests := []struct {
name string
osIdentifier string
collectors []string
wantErr bool
name string
collectors []string
wantErr bool
}{
{
name: "windows",
osIdentifier: "windows",
collectors: []string{"SMI"},
wantErr: true,
name: "no OSM or SMI included",
collectors: []string{"NOT_OSM", "NOT_SMI"},
wantErr: true,
},
{
name: "linux without OSM or SMI included",
osIdentifier: "linux",
collectors: []string{"NOT_OSM", "NOT_SMI"},
wantErr: true,
name: "only OSM included",
collectors: []string{"OSM", "NOT_SMI"},
wantErr: false,
},
{
name: "linux with OSM included",
osIdentifier: "linux",
collectors: []string{"OSM", "NOT_SMI"},
wantErr: false,
},
{
name: "linux with SMI included",
osIdentifier: "linux",
collectors: []string{"NOT_OSM", "SMI"},
wantErr: false,
name: "only SMI included",
collectors: []string{"NOT_OSM", "SMI"},
wantErr: false,
},
}

for _, tt := range tests {
runtimeInfo := &utils.RuntimeInfo{
OSIdentifier: tt.osIdentifier,
CollectorList: tt.collectors,
}
c := NewSmiCollector(nil, runtimeInfo)
Expand Down Expand Up @@ -84,7 +73,6 @@ func TestSmiCollectorCollect(t *testing.T) {
}

runtimeInfo := &utils.RuntimeInfo{
OSIdentifier: "linux",
CollectorList: []string{"SMI"},
}

Expand Down
17 changes: 0 additions & 17 deletions pkg/utils/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,23 +200,6 @@ func GetCreationTimeStamp(config *restclient.Config) (string, error) {
return creationTimeStamp, nil
}

// GetResourceList gets a list of all resources of given type in a specified namespace
func GetResourceList(kubeCmds []string, separator string) ([]string, error) {
outputStreams, err := RunCommandOnContainerWithOutputStreams("kubectl", kubeCmds...)

if err != nil {
return nil, err
}

resourceList := outputStreams.Stdout
// If the resource is not found within the cluster, then log a message and do not return any resources.
if len(resourceList) == 0 {
return nil, fmt.Errorf("no '%s' resource found in the cluster for given kubectl command", kubeCmds[1])
}

return strings.Split(strings.Trim(resourceList, "\""), separator), nil
}

func GetPods(clientset *kubernetes.Clientset, namespace string) (*v1.PodList, error) {
// Create a pod interface for the given namespace
podInterface := clientset.CoreV1().Pods(namespace)
Expand Down