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

Adding Vsphere Functionality to deployment #489

Open
wants to merge 103 commits into
base: main
Choose a base branch
from

Conversation

tosin2013
Copy link

Description

Adding vShpere functionality to integrate with the following repo https://github.com/Red-Hat-SE-RTO/openshift-ztp.

Fixes # (issue)

Type of change

Please select the appropriate options:

  • Bug fix (non-breaking change which fixes an issue)
  • [X ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [X ] This change requires a documentation update
  • This change is a documentation update

Testing

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Test A
  • Test B

Test Configuration:

  • Versions:
  • Hardware:

Checklist

  • I have performed a self-review of my own code
  • If a change is adding a feature, it should require a change to the README.md and the review should catch this.
  • If the change is a fix, it should have an issue. The review should make sure the comments state the issue (not just the number) and it should use the keywords that will close the issue on merge.
  • A change should not be merged unless it passes CI or there is a comment/update saying what testing was passed.
  • PRs should not be merged unless positively reviewed.

@alknopfler
Copy link
Contributor

ready to review or draft yet?

@tosin2013
Copy link
Author

ready to review or draft yet?

I am working on the code now I will have a draft of the steps later today.

.github/workflows/build-and-push-release.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Comment on lines 27 to 38
OC_COMMAND=$(oc get csv -n ansible-automation-platform | grep "Ansible Automation Platform" | grep Succeeded | wc -l)
timeout=0
while [ "${timeout}" -lt "120" ]; do
if [[ ${OC_COMMAND} -eq 1 ]]; then
ready=true
break
fi
echo "Wait for Ansible Automation Platform"
sleep 5
timeout=$((timeout + 1))
OC_COMMAND=$(oc get csv -n ansible-automation-platform | grep "Ansible Automation Platform" | grep Succeeded | wc -l)
done
Copy link
Contributor

Choose a reason for hiding this comment

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

In the common.sh file there is a check_resources which do exactly this part. An example:

echo "Installing NFD operator for ${edgecluster}"
        oc --kubeconfig=${EDGE_KUBECONFIG} apply -f manifests/01-nfd-namespace.yaml
        sleep 2
        oc --kubeconfig=${EDGE_KUBECONFIG} apply -f manifests/02-nfd-operator-group.yaml
        sleep 2
        oc --kubeconfig=${EDGE_KUBECONFIG} apply -f manifests/03-nfd-subscription.yaml
        sleep 2

        check_resource "crd" "nodefeaturediscoveries.nfd.openshift.io" "Established" "openshift-nfd" "${EDGE_KUBECONFIG}"

Comment on lines 46 to 57
echo ">>>> Wait until acm ready"
echo ">>>>>>>>>>>>>>>>>>>>>>>>>"
timeout=0
ready=false
sleep 240
while [ "${timeout}" -lt "120" ]; do
if [[ $(oc get pod -n ansible-automation-platform | grep ac- | grep -i running | wc -l) -eq $(oc get pod -n ansible-automation-platform | grep ac- | wc -l) ]]; then
ready=true
break
fi
sleep 5
timeout=$((timeout + 1))
Copy link
Contributor

Choose a reason for hiding this comment

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

same than before

# Load common vars
source ${WORKDIR}/shared-utils/common.sh

echo "development"
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to make it clear, the idea of the contrib framework is to have an interface with 2 functions:

  • Deploy.sh -> deploy resources for a specific resource/operator/item
  • Verify.sh -> verify if there's present to skip this task. The verification should be a functional test to know if the resources are ready and running.

So, said that, in this part an example of verify could be:

Verify.sh

@@ -0,0 +1,17 @@
#!/usr/bin/env bash
Copy link
Contributor

Choose a reason for hiding this comment

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

same than before ;)

@@ -0,0 +1,4 @@
kind: Namespace
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need gitea? Maybe I'm wrong or missing something but I think we don't need argocd to deploy it

Copy link
Author

Choose a reason for hiding this comment

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

gitea is used for organizations that do not have access to github.com or bitbucket to stand up a repo and point to this one https://github.com/Red-Hat-SE-RTO/openshift-ztp. There are some users that have proxy issues or firewall issues that do not allow them to access git repo.

@@ -0,0 +1,94 @@
apiVersion: tekton.dev/v1beta1
Copy link
Contributor

Choose a reason for hiding this comment

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

this file should be placed on folder contrib like:
https://github.com/rh-ecosystem-edge/ztp-pipeline-relocatable/tree/33f65288208908623e96fd82269bd381c0ba6560/pipelines/resources/contrib
this is the way to use contrib resources in our pipeline

Copy link
Contributor

Choose a reason for hiding this comment

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

also there, you've got a template file created just to copy and paste modifying the resources and deployments you need to use in your case

@@ -8,5 +8,6 @@ resources:
- deploy-ztp-hub-mce.yaml
- deploy-ztp-edgeclusters.yaml
- deploy-ztp-edgeclusters-sno.yaml
- deploy-ztp-vsphere.yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

same the folder is contrib/

@@ -88,9 +88,15 @@ fi

echo ">>>> Verify oc get nodes"
echo ">>>>>>>>>>>>>>>>>>>>>>>>>"
if [[ $(oc get nodes | grep -i ready | wc -l) -ne 1 ]] && [[ $(oc get nodes | grep -i ready | wc -l) -ne 3 ]]; then
# TO-DO: Installed Vshpere with assisted installer and enable vshpere during deployment
Copy link
Contributor

Choose a reason for hiding this comment

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

don't understand this comment TO-DO, why do you need to install vsphere with AI?

Copy link
Author

Choose a reason for hiding this comment

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

Right one my vSphere cluster has 3 masters and 3 workers so it is failing on the check. I wanted to add the vShpere check because I know that baremetal can be sno or 3 nodes. vShpere can have more that 3 worker nodes in some environments.

@alknopfler
Copy link
Contributor

First of all thanks @tosin2013 for the contribution, I've done a previous review just to give you some feedback. If you need help or make something more clear just ping me ok?

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Aug 11, 2022
@github-actions
Copy link

This pull request has conflicts, please resolve those before we can evaluate the pull request.

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

Successfully merging this pull request may close these issues.

2 participants