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

Kind 0.8.1 devenv #978

Merged
merged 3 commits into from
May 22, 2020
Merged

Conversation

dgoodwin
Copy link
Contributor

@dgoodwin dgoodwin commented May 7, 2020

New script is based off an example in kind.

Been trying to get this to work but it's stuck communicating with the insecure registry, no route to host despite supposedly being on the network. I've exec'd into the kind control plane container and using curl, neither localhost, nor the container name (kind-registry), nor the ip seem to work. It just can't reach that container.

Any advice welcome, going to keep trying to poke at this.

Still haven't tried it with podman.

Fixes: #833

@openshift-ci-robot openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 7, 2020
@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 7, 2020
@dgoodwin
Copy link
Contributor Author

dgoodwin commented May 8, 2020

Script seems to work once I made two Fedora 32 hacks to fix docker container networking in general:

  1. sudo firewall-cmd --permanent --zone=trusted --add-interface=docker0
  2. change the FirewallBackend in the /etc/firewalld/firewalld.conf file from nftables to iptables and restart docker ([firewalld] kind doesn't work on Fedora 32 kubernetes-sigs/kind#1547 (comment))

So this script replaces both of our current ones for kind and registry into one much smaller and simpler script.

Next step podman.

@dgoodwin
Copy link
Contributor Author

dgoodwin commented May 8, 2020

hive-operator appears to be successful, but hotloops and pins the cpu. Investigating.

@dgoodwin
Copy link
Contributor Author

Hotloop fixed, details in commit, some interesting complications may be coming with Kube 1.18+ due to server side apply.

Updated docs.

```

Create a kind cluster named 'hive' to deploy to. You can create as many kind clusters as you need.:
Create a local insecure registry (if one does not already exist) and then a kind cluster named 'hive' to deploy to. You can create as many kind clusters as you need and you have sufficient RAM.

```bash
./hack/create-kind-cluster.sh hive
Copy link
Contributor

Choose a reason for hiding this comment

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

Add a note after this that the following error is expected the first time this is run and is not indicative of an error.

Error response from daemon: container fcca36a26da5601d6453c0c53ab5909eb6ca8ffe42de0f8634dd7213f107cef0 is not connected to network kind

"
EOF
fi
# tell https://tilt.dev to use the registry
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this section required? If so, we should remove the references to tilt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't even realize, no it does not seem necessary, removing.

```

Create a kind cluster named 'hive' to deploy to. You can create as many kind clusters as you need.:
Create a local insecure registry (if one does not already exist) and then a kind cluster named 'hive' to deploy to. You can create as many kind clusters as you need and you have sufficient RAM.
Copy link
Contributor

Choose a reason for hiding this comment

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

The part about sufficient RAM could use some grammatical work. I do not think that it is necessary to call out specifically RAM.

Suggested change
Create a local insecure registry (if one does not already exist) and then a kind cluster named 'hive' to deploy to. You can create as many kind clusters as you need and you have sufficient RAM.
Create a local insecure registry (if one does not already exist) and then a kind cluster named 'hive' to deploy to. You can create multiple kind clusters. Each kind cluster needs a unique name, which is specified by the argument passed to the script.

@dgoodwin dgoodwin changed the title WIP: Kind 0.8.1 devenv Kind 0.8.1 devenv May 11, 2020
@openshift-ci-robot openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 11, 2020
Switch to a much simpler script for creating a kind cluster to develop
hive against, with an integrated registry. The registry is now linked to
the container with docker networking and the new script takes place of
both of the old ones, if there is no registry container one will be
started for you.
1.18 introduces beta of server side apply, which adds field tracking to
object metadata for who set the value of a field. On Update, this
includes a timestamp. Hive operator was blindly doing updates and thus
the timestamp was constantly changing, leading to a hotloop.
@dgoodwin
Copy link
Contributor Author

Updates pushed and WIP removed. Matthew and Joel have confirmed this works for them so I think we're ok to proceed.

Added some additional notes on the Fedora hacks that may be required.

@dgoodwin
Copy link
Contributor Author

Needs approval, should be ready to go.

Copy link
Contributor

@staebler staebler left a comment

Choose a reason for hiding this comment

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

Bah, had one very minor comment that I never submitted from a week ago. Otherwise, this looks good to me.


Create a kind cluster named 'hive' to deploy to. You can create as many kind clusters as you need.:
Create a local insecure registry (if one does not already exist) and then a kind cluster named 'hive' to deploy to. You can create additional clusters if desired by provising a different name argument to the script.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/provising/providing/


```bash
export KUBECONFIG="$(kind get kubeconfig-path --name="hive")"
Copy link
Contributor

Choose a reason for hiding this comment

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

The new kind does not have get kubeconfig-path so also need to change the docs in Adopting clusterdeployments section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I replaced it with /path/to/admin/kubeconfig.

@dgoodwin
Copy link
Contributor Author

PR updated, would love to get merged before freeze!

@@ -130,7 +129,7 @@ To create a kind cluster and adopt:

```bash
./hack/create-kind-cluster.sh cluster1
bin/hiveutil create-cluster --base-domain=new-installer.openshift.com kind-cluster1 --adopt --adopt-admin-kubeconfig=$(kind get kubeconfig-path --name="cluster1") --adopt-infra-id=fakeinfra --adopt-cluster-id=fakeid
bin/hiveutil create-cluster --base-domain=new-installer.openshift.com kind-cluster1 --adopt --adopt-admin-kubeconfig=/path/to/cluster/admin/kubeconfig get kubeconfig-path --name="cluster1") --adopt-infra-id=fakeinfra --adopt-cluster-id=fakeid
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the following.

get kubeconfig-path --name="cluster1")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arghhh sorry, updated again.

Copy link
Contributor

@staebler staebler left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label May 21, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgoodwin, staebler

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

/retest

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

2 similar comments
@openshift-bot
Copy link

/retest

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

@openshift-bot
Copy link

/retest

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

@openshift-merge-robot openshift-merge-robot merged commit 39d1b4f into openshift:master May 22, 2020
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update hack scripts to work with latests versions of Kind
6 participants