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

Refactor docs to address #14, and add a gitops job #15

Merged
merged 5 commits into from
Jun 9, 2020

Conversation

etsauer
Copy link
Contributor

@etsauer etsauer commented Jun 9, 2020

Copy link
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

A couple comments and requested changes


The simple cluster bootstrapping example shows how cluster administrators might begin managing OpenShift clusters using just `oc apply`. Each resource in this example carries a common label (`example.com/project: simple-bootstrap`) that associates it with this `project`. In doing this, we can manage the full lifecycle of our resources with a single command.

```
oc apply -Rf simple-bootstrap/ --prune -l example.com/project=simple-bootstrap
until oc apply -Rf simple-bootstrap/ --prune -l example.com/project=simple-bootstrap; do sleep 2; done
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to send stderr to /dev/null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if we do that, and the loop gets hung, the user would never see any ouput. I don't think I'm that worried about having the output "clean". Probably better to show everything and explain that errors will be expecte when hitting race conditions

userconfig.redhatcop.redhat.io/sandboxes created
```

If we run this a second time, we'll see that it still completes successfully, but notice that the action taken to each file has been changed from `create` to `unchanged` or in some cases `configured`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Given the until loop previously executed, there is a high likelihood users would have already seen unchanged execution

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's fine. In this section we're just talking about what would happen if you had only run oc apply, not the entire loop

README.md Outdated

However, there's one likely hiccup that our workflow needs to be able to handle. The management of operators via the [Operator Lifecycel Manager](https://github.com/operator-framework/operator-lifecycle-manager) creates a race condition. When a `Subscription` and `OperatorGroup` resource gets created, it triggers OLM to fetch details about the operator, and install the relevant `CustomResourceDefinitions`(CRDs). Until the CRDs have been put to the cluster, an attempt to create a matching `CustomResource` will fail, as that resource type doesn't yet exist in the API.

In our case, we are deploying the [Namespace Configuration Operator](https://github.com/redhat-cop/namespace-configuration-operator), which provides the `UserConfig` resource type. If we try to create both the `OperatorGroup`/`Subscription` to deploy the operator, and the `UserConfig` to invoke it in the same comman, we'll get an error:
Copy link
Contributor

Choose a reason for hiding this comment

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

comman -> command

README.md Outdated
By running the workflow locally, we've already created a `CronJob` in the `cluster-ops` namespace. In order for it to run, it requires a secret be created pointing it to the repository where the cluster configs live.

```
oc create secret generic gitops-repo --from-literal=url=https://github.com/redhat-cop/declarative-openshift.git --from-literal=contextDir=simple-bootstrap --from-literal=pruneLabel=example.com/project=simple-bootstrap -n cluster-ops
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason for creating a Secret instead of a ConfigMap?

Copy link
Contributor Author

@etsauer etsauer Jun 9, 2020

Choose a reason for hiding this comment

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

so that we are set up to include credentials for private repos in the future.

if [ -z "$var" ]; then
echo "Syncing cluster config from $GIT_REPO/$CONTEXT_DIR"
git clone $GIT_REPO /tmp/repodir
until oc apply -Rf /tmp/repodir/$CONTEXT_DIR/ --prune -l $PRUNE_LABEL $(cat /tmp/repodir/prune-whitelist.txt); do sleep 2; done
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the API Group for DaemonSet be changed to apps/v1 as extensions/v1beta1 which causes the job to not complete in OCP 4.4

@etsauer
Copy link
Contributor Author

etsauer commented Jun 9, 2020

@sabre1041 updated.

- |
if [ -z "$var" ]; then
echo "Syncing cluster config from $GIT_REPO/$CONTEXT_DIR"
git clone $GIT_REPO /tmp/repodir
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a parameter to clone a specific branch?

@@ -7,7 +7,7 @@
--prune-whitelist core/v1/Service
--prune-whitelist batch/v1/Job
--prune-whitelist batch/v1beta1/CronJob
--prune-whitelist extensions/v1beta1/DaemonSet
--prune-whitelist apps/v1/DaemonSet
--prune-whitelist extensions/v1beta1/Deployment
Copy link
Contributor

Choose a reason for hiding this comment

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

Deployment: extensions/v1beta1 -> apps/v1
ReplicaSet: extensions/v1beta1 -> apps/v1

Copy link
Contributor

@sabre1041 sabre1041 left a comment

Choose a reason for hiding this comment

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

LGTM

@sabre1041 sabre1041 merged commit 70e081e into redhat-cop:master Jun 9, 2020
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.

Document how to handle CRD/CR race condition
2 participants