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

Replace pip installation with APT for python-openshift #114

Merged
merged 2 commits into from
Mar 6, 2021

Conversation

anthr76
Copy link
Member

@anthr76 anthr76 commented Mar 2, 2021

Description

Let python-openshift be managed with APT instead of PIP. I'm maintaining a repo for various debian flavors but at the moment 20.04 is hardcoded. We'll remain to pull in from the AUR for arch linux.

Checklist

  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • All commits contain a well written commit description including a title, description and a Fixes: #XXX line if the commit addresses a particular GitHub issue.
  • All workflow validation and compliance checks are passing.

Notes

Resolves tech debt from #72


- name: 'install openshift kubernetes module (3/3)'
apt:
name: python-openshift
Copy link
Member

Choose a reason for hiding this comment

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

Do we use this anywhere?

Copy link
Member Author

Choose a reason for hiding this comment

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

We do in Cilium.

community.kubernetes.helm_repository:
name: cilium
repo_url: "https://helm.cilium.io/"
- name: deploy Cilium
community.kubernetes.helm:
name: cilium
chart_ref: cilium/cilium
release_namespace: kube-system
chart_version: "{{ cni_cilium_helm_version }}"
values: "{{ lookup('template', 'values.yaml.j2') | from_yaml }}"
- name: patch cilium-operator for helm chart bug
community.kubernetes.k8s:
state: present
definition:
apiVersion: apps/v1
kind: Deployment
metadata:
name: cilium-operator
namespace: kube-system
spec:
template:
spec:
containers:
- name: cilium-operator
image: cilium/operator-dev:{{ cni_cilium_image_version }}
- name: Apply kube-router manifests
community.kubernetes.k8s:

Likely elsewhere if the community is in agreeance, and sees fit for other CNI deployments etc

Copy link
Member Author

Choose a reason for hiding this comment

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

commit: 0c527dc

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this is purely in usage for the CNI should we keep it here or move it to the respective role?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. If we slot in CNI role we'll have to do distro based checks. The equivalent to this for Arch is also in common. In my opinion it would make sense here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to add on to this tidbit. If we were to move forward with issues like this it would make more sense to keep this in common.

@anthr76
Copy link
Member Author

anthr76 commented Mar 2, 2021

Copy link
Member

@xunholy xunholy left a comment

Choose a reason for hiding this comment

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

I'm happy with this change, will wait to let @rkage to leave any further feedback.

* Remove pip as a common package and replace it with `python-openshift`
from OBS
* Add OBS key before installing common packages to ensure
`python-openshift` gets pulled in.
* Since an AUR package exists for `python-openshift` remove pip from
Arch and install from AUR.

Signed-off-by: anthr76 <[email protected]>
@anthr76 anthr76 merged commit 0e87edb into main Mar 6, 2021
@anthr76 anthr76 deleted the openshift-module branch March 6, 2021 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants