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

Conversation

peterbom
Copy link
Contributor

Now that the OSM and SMI collectors are no longer dependent on kubectl, we can remove the binary from the Periscope image.

This removes it from the Docker image, along with other unused packages (taking the Linux image size from 161MB to 63MB).

It also updates the OSM and SMI collectors so that they will be enabled on Windows nodes.

To test manually, run Periscope from VS Code after setting the following in settings.json:

    "aks.periscope.containerRegistry": "ghcr.io/peterbom",
    "aks.periscope.imageVersion": "0.0.9",
    "aks.periscope.releaseTag": "v0.9",
    "aks.periscope.repoOrg": "peterbom",

@peterbom peterbom requested a review from Tatsinnit June 11, 2022 03:56
@codecov-commenter
Copy link

Codecov Report

Merging #181 (4311a99) into master (2377183) will decrease coverage by 0.11%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #181      +/-   ##
==========================================
- Coverage   78.94%   78.82%   -0.12%     
==========================================
  Files          13       13              
  Lines         717      713       -4     
==========================================
- Hits          566      562       -4     
  Misses         92       92              
  Partials       59       59              
Impacted Files Coverage Δ
pkg/collector/osm_collector.go 75.41% <ø> (-0.21%) ⬇️
pkg/collector/smi_collector.go 78.46% <ø> (-0.65%) ⬇️

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 2377183...4311a99. Read the comment docs.

@Tatsinnit Tatsinnit added the enhancement 🏎 New feature or request label Jun 11, 2022
@@ -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

@Tatsinnit Tatsinnit marked this pull request as ready for review June 12, 2022 23:18
@Tatsinnit Tatsinnit merged commit 8bd3620 into Azure:master Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🏎 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants