-
Notifications
You must be signed in to change notification settings - Fork 29
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
Ingress Firewall Operator Automation #100
base: main
Are you sure you want to change the base?
Ingress Firewall Operator Automation #100
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Yashansh-Sharma15 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Please squash the multiple commits and add your Signed-off-by to the commit. |
00b56de
to
40e69ba
Compare
playbooks/roles/ocp-ingress-firewall-operator/default/main.yaml
Outdated
Show resolved
Hide resolved
40e69ba
to
eafcc6a
Compare
seconds: 30 | ||
|
||
- name: Verify creation of Catsrc | ||
shell: " oc get catsrc -A | grep qe-app-registry" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we using the variable ingress_catalogsource_name
instead to hardcoded value - qe-app-registry
failed_when: catsrc.rc != 0 | ||
|
||
- name: Verify creation of pods | ||
shell: "oc get pods -n openshift-marketplace | grep qe-app-registry | grep Running" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here - shouldn't we using the variable ingress_catalogsource_name
instead to hardcoded value - qe-app-registry
spec: | ||
name: ingress-node-firewall | ||
channel: stable | ||
source: qe-app-registry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here - shouldn't we using the variable ingress_catalogsource_name
instead to hardcoded value - qe-app-registry
- eth0 | ||
nodeSelector: | ||
matchLabels: | ||
<ingress_firewall_label_name>: <label_value> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These placeholders should either be defined as variables or explicitly documented in README.
- eth1 | ||
nodeSelector: | ||
matchLabels: | ||
<ingress_firewall_label_name>: <label_value> # Replace with actual label selectors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also here - These placeholders should either be defined as variables or explicitly documented in README.
|
||
# Delete the Dual operator group | ||
- name: Delete Ingress Node Firewall OperatorGroup | ||
shell: "oc delete og ingress-node-firewall-operators -n openshift-ingress-node-firewall" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we do the cleanup in the same way we create the OperatorGroup, by using kubernetes.core.k8s
with state: absent
? It would make the process more declarative and consistent.
Signed-off-by: Yashansh-Sharma15 <[email protected]>
eafcc6a
to
ea12084
Compare
This is the Playbook for automation of ingress firewall operator.