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

Support --buid-arg for upstream repos #3

Closed
wants to merge 1 commit into from

Conversation

alexlovelltroy
Copy link

This PR alone will not fully address issue #2. More work is needed to make the liveness python library available externally and to then deprecate the --index-url flags from the requirements.txt files. It's a start though. Use build args like:

docker build -t cfs-operator-test --build-arg BASE_CONTAINER=alpine:3.12.0 --build-arg PIP_INDEX_URL=https://pypi.python.org/simple/ -f Dockerfile .

Keep in mind that in Dockerfiles the ARGS are cleared at each FROM line which is why they are defined multiple times in the same file.

@rkleinman-hpe
Copy link
Contributor

What needs to happen for #2 (specifically the python package) to be available externally?

@alexlovelltroy
Copy link
Author

I think we need to publish k8s-liveness to pypi or flip the repo to public and reference the github repo in requirements.txt

@rkleinman-hpe
Copy link
Contributor

I vote for the latter, should be simple to do.

@alexlovelltroy
Copy link
Author

I'd like @tpletcher-hpe to weigh in about referencing a github repo in requirements.txt and how he feels about the software supply chain on that.

@rkleinman-hpe
Copy link
Contributor

It will be problematic or will require some build time tooling to switch between referencing the github repo vs the internal repo until we have just one repo to rule them all.

@rkleinman-hpe
Copy link
Contributor

I have incorporated the changes for this into another PR that has now been merged internally, commit 8eec44b should have everything and will likely create a conflict with this PR. Suggest closing.

@rkleinman-hpe
Copy link
Contributor

Changes have been incorporated on the HPE side. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants