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

[Enhancement] sd-step should check local hab dir first before installing #1163

Closed
minzcmu opened this issue Jun 29, 2018 · 10 comments · Fixed by screwdriver-cd/sd-step#13
Closed
Labels

Comments

@minzcmu
Copy link
Member

minzcmu commented Jun 29, 2018

What happened:
We're getting throttling by habitat.sh when there're a lot of files to upload in sd-teardown-artifacts-bookend. Because every time we run sd-step, we makes a call to fetch all the versions first and find the match.
https://github.com/screwdriver-cd/sd-step/blob/master/sd-step.go#L185

Example error:

00:03:30   ERROR: Failed to get package version: Failed to fetch package versions: Get https://willem.habitat.sh/v1/depot/pkgs/core/curl?range=0: net/http: request canceled (Client.Timeout exceeded while awaiting headers)

What you expected to happen:
We shouldn't keep calling that endpoint. Can check if there is a local hab pkg matches the semver first before making the call to fetch versions.

@catto @kumada626 Can you guys help review/investigate this issue?

Updates
Actually also seeing this err in our functional tests and that only uploads 3 files... https://beta.cd.screwdriver.cd/pipelines/375/builds/22032

Even me hitting that endpoint in my browser got timeout: https://willem.habitat.sh/v1/depot/pkgs/core/curl?range=0
it's intermittent
screen shot 2018-06-29 at 3 08 39 pm

@jithine
Copy link
Member

jithine commented Jun 29, 2018

Whether this is a habitat API issue or not, we could modify sd-cmd to not be make extra API calls when it is not required. After initial use of SD command for curl we already have the hab command locally, subsequent commands could use this without having to reach out to habitat.

@jithine jithine added the bug label Jun 29, 2018
@catto
Copy link
Member

catto commented Jun 30, 2018

Ah I didn't noticed this issue and just created new one #1164 🚶...
@minz1027 I agree with that. We should not call depot API every time even if there is already a package that a user want to run.
@jithin1987 sd-cmd rarely makes API calls. Seems good with fixing sd-step only.

@kumada626
Copy link
Contributor

sd-cmd already checks whether the hab command is downloaded or not.
https://github.com/screwdriver-cd/sd-cmd/blob/master/executor/habitat.go#L104
I agree with reducing API calls in the sd-step as well.

@catto
Copy link
Member

catto commented Jul 4, 2018

Should we avoid using curl in artifact-bookend? It might be good idea to implement a small command for uploading artifact without curl, place it to /opt/sd/bin and call it from bookend.

@catto
Copy link
Member

catto commented Jul 5, 2018

Additionally, later versions of core/curl cannot run on older kernel such as centos 6. I think it is because of glibc compatibility.

> core/curl/7.54.1/20180608142121
$ hab pkg exec core/curl curl
FATAL: kernel too old
> core/curl/7.54.1/20171101045605
$ hab pkg exec core/curl curl
curl: try 'curl --help' for more information

There seems to be few people using such an OS especially for k8s, but we have k8s-vm executor that can run older OSes on VM by a user demand.
I think we should use core/curl-static-musl instead of core/curl tentatively

@minzcmu
Copy link
Member Author

minzcmu commented Jul 6, 2018

@catto Hi Catto, we tested out the latest sd-step. It seems like it doesn't reduce the number of API calls to get versions. Based on the logic, every time sd-step is called, it will still call getPackageVersion and make a hab api call to list all versions. And then if that failed, it will use local hab pkg.
https://github.com/screwdriver-cd/sd-step/pull/13/files#diff-f2e08b845a06febf8f0440cb7790e8baR117

To really reduce the calls to list versions, we should check the local hab first before calling depot.PackageVersionsFromName

@minzcmu minzcmu reopened this Jul 6, 2018
@catto
Copy link
Member

catto commented Jul 9, 2018

@minz1027 Sorry, I forgot to update the progress and did not notice that this issue was closed automatically. API call did not reduce because curl command in artifact-bookend calls with --pkg-version arg. I am going to fix this with checking a command installed with specified version as non-semver expression before the API call (here)

@catto
Copy link
Member

catto commented Jul 9, 2018

Update(2018/07/09): Finally reduced API calls from sd-step command. I confirmed there is no access to habitat API if a package already exists in /hab with the latest sd-step.
TODO

  • Rebuild launcher to pull in the new sd-step command
  • Test with the latest launcher

@d2lam
Copy link
Member

d2lam commented Jul 9, 2018

Thanks @catto, please let us know after you're done testing. We'll pull it in as well.

@catto
Copy link
Member

catto commented Jul 10, 2018

@d2lam I confirmed that the latest sd-step works fine and API calls are reduced. You can use it by pulling launcher v4.0.121 to your environment.

@catto catto closed this as completed Jul 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants