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

hack: add a development way to run the operator as part of the installer #94

Merged
merged 1 commit into from
Feb 11, 2019
Merged

Conversation

squeed
Copy link
Contributor

@squeed squeed commented Feb 8, 2019

This is a stupid way to tell the installer not to run the network operator, and let us run it manually as part of the install process.

@openshift-ci-robot openshift-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 8, 2019
@squeed squeed requested review from dcbw and removed request for danwinship and knobunc February 8, 2019 18:18
@pecameron
Copy link
Contributor

@squeed Is this the way to develop the ovnkube type? I install and by the time I get to log in networking is setup and fixed. Can't change type.

Copy link
Contributor

@danwinship danwinship left a comment

Choose a reason for hiding this comment

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

we should get some comments from people who know more about how the installer works to make sure this is all sane?

INSTALLER-HACKING.md Outdated Show resolved Hide resolved
INSTALLER-HACKING.md Outdated Show resolved Hide resolved
INSTALLER-HACKING.md Show resolved Hide resolved
hack/run-locally.sh Outdated Show resolved Hide resolved
hack/run-locally.sh Outdated Show resolved Hide resolved
hack/run-locally.sh Outdated Show resolved Hide resolved
hack/run-locally.sh Outdated Show resolved Hide resolved
hack/run-locally.sh Outdated Show resolved Hide resolved
hack/run-locally.sh Outdated Show resolved Hide resolved
@squeed
Copy link
Contributor Author

squeed commented Feb 11, 2019

Updated, now with 100% more cleverness. Now we automatically extract the image references.

.gitignore Outdated Show resolved Hide resolved
After destroying your cluster, you should delete your `$CLUSTER_DIR`. If you're using libvirt, you can use `./scripts/maintenance/virsh-cleanup.sh` in the installer repository.

### RBAC
This runs the operator with the `admin` role, so it will have full permissions. When run as the real installer, this will not be the case. So you can't debug RBAC issues here.
Copy link
Contributor

Choose a reason for hiding this comment

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

"when run by the real installer"?

FWIW we're going to have to figure out how to fix this fairly quickly, since part of the OVN work will be making sure we have RBAC set up correctly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This only applies to the operator process. Any ovn processes will still go through rbac.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, "run as" -- German prepositions are leaking out again.

INSTALLER-HACKING.md Outdated Show resolved Hide resolved
hack/run-locally.sh Show resolved Hide resolved
# Patch the CVO to not create the network operator
echo "Applying overrides to ${CLUSTER_DIR}/manifests/cvo-overrides.yaml"

kubectl --kubeconfig=hack/null-kubeconfig patch --type=json --local=true -f=${CLUSTER_DIR}/manifests/cvo-overrides.yaml -p "$(cat hack/overrides-patch.yaml)" -o yaml > ${CLUSTER_DIR}/manifests/.cvo-overrides.yaml.new
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're requiring oc elsewhere, change this to oc for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For some reason this doesn't work with oc. I don't remember why.

@squeed
Copy link
Contributor Author

squeed commented Feb 11, 2019

OK, updated.

@danwinship
Copy link
Contributor

Updated, now with 100% more cleverness. Now we automatically extract the image references.

Ah, I had meant using the in-tree daemonset.yaml, not the one from the image, which is what you're using I guess? In particular, I was thinking about the fact that for OVN, the operator will also need to have $OVN_IMAGE set.

Maybe just add a comment about this to step 3?

@squeed
Copy link
Contributor Author

squeed commented Feb 11, 2019

Mmm, I was thinking of a different use case. That's starting to sound like actual code ;-).

@squeed
Copy link
Contributor Author

squeed commented Feb 11, 2019

Wait nevermind, that's really easy and obviously the right thing to do. Hold please.

This is a stupid way to tell the installer not to run the network
operator, and let us run it manually as part of the install process.
@squeed
Copy link
Contributor Author

squeed commented Feb 11, 2019

That makes it infinitely simpler, and you don't need a new oc. Updated.

# Extract the image references from the release image.
function build_env() {
echo "Writing image references to ${CLUSTER_DIR}/env.sh"
oc --kubeconfig=hack/null-kubeconfig convert --local=true -f manifests/0000_07_cluster-network-operator_03_daemonset.yaml -ojsonpath='{range .spec.template.spec.containers[0].env[?(@.value)]}{.name}{"="}{.value}{"\n"}' > ${CLUSTER_DIR}/env.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

oh. heh. I had assumed you had some reason for extracting the exact version numbers of out of the built image :-)

export KUBERNETES_SERVICE_HOST=$(oc config view -o jsonpath='{.clusters[0].cluster.server}' | awk -F'[/:]' '{print $4}')

echo "Waiting for the apiserver to come up and for the network configuration to be rceated."
echo "You can ignore the error message 'error: the server doesn't have a resource type \"Network\"'"
Copy link
Contributor

Choose a reason for hiding this comment

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

You can ignore the error message 'error: the server doesn't have a resource type "Network"'
The connection to the server danwinship-api.devcluster.openshift.com:6443 was refused - did you specify the right host or port?

Anyway, we can make further tweaks to the instructions later

@danwinship
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Feb 11, 2019
@openshift-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: danwinship, squeed

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link
Contributor

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 38a77fe into openshift:master Feb 11, 2019
@squeed squeed deleted the hack-installer branch February 12, 2019 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants