Skip to content

HDDS-4969. Extract check dependency installation from Github Actions workflow#2029

Merged
elek merged 8 commits intoapache:masterfrom
adoroszlai:HDDS-4969
Apr 8, 2021
Merged

HDDS-4969. Extract check dependency installation from Github Actions workflow#2029
elek merged 8 commits intoapache:masterfrom
adoroszlai:HDDS-4969

Conversation

@adoroszlai
Copy link
Contributor

What changes were proposed in this pull request?

Move installation of external dependencies for some CI checks from Github Actions workflow to the script of the checks. This reduces complexity of Github-specific code in the workflow definition, and allows simpler prototyping/testing.

https://issues.apache.org/jira/browse/HDDS-4969

How was this patch tested?

https://github.com/adoroszlai/hadoop-ozone/actions/runs/643852771

@adoroszlai adoroszlai self-assigned this Mar 11, 2021
@adoroszlai adoroszlai requested a review from elek March 11, 2021 20:27
@adoroszlai
Copy link
Contributor Author

@elek this is based on your idea from #1984. Installation of spotbugs is not included here because it is still run inside the builder image, which already has the tool.

If this is approved, I'll rebase the other PR.

@adoroszlai adoroszlai added the build Pull request that modifies the build process label Mar 12, 2021
Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

Thanks for the patch @adoroszlai. I have a few suggestions (or more like some questions what I would like to discuss...)

Copy link
Member

Choose a reason for hiding this comment

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

I like this one (my local install always has higher priority) but in this case if I have bats on my own PATH, we don't need to download the new one...

I don't know what is the best approach, but we may need to skip the download/install of the binaries if they are already available...

Maybe we can switch to a tool based directory hierarchy (.dev-tools/k3s/...) instead of check based hierarchy (.dev-tools/acceptance/...). It would also help us to reuse tools if they are used from multiple checks. (but with matrix build it may not be required any more...).

Sg. like _install_tool <binary_name> <callback> but you may have a better idea for a good abstraction....

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export KUBECONFIG=/etc/rancher/k3s/k3s.yaml
: ${KUBECONFIG:=/etc/rancher/k3s/k3s.yaml}

Just to support minikube and other users....

Copy link
Member

Choose a reason for hiding this comment

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

You can use something like this to avoid sudo:

 virtualenv "${TOOL_DIR}"/robotenv
 source "${TOOL_DIR}"/robotenv/bin/activate
 pip install robotframework

and you can add "${TOOL_DIR}"/robotenv/bin to the path

@elek
Copy link
Member

elek commented Mar 22, 2021

One more comment: we may need to add .dev-tools to .gitignore...

@mukul1987 mukul1987 force-pushed the master branch 2 times, most recently from 79a9d39 to 520ba00 Compare March 25, 2021 16:05
Copy link
Member

@elek elek left a comment

Choose a reason for hiding this comment

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

+1, thanks the update @adoroszlai

@elek elek merged commit 4f76d03 into apache:master Apr 8, 2021
@adoroszlai adoroszlai deleted the HDDS-4969 branch April 8, 2021 12:24
@adoroszlai
Copy link
Contributor Author

Thanks @elek for reviewing and committing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Pull request that modifies the build process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments