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.

Sign up for GitHub

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

Jump to bottom

Backport Pin controller-runtime to v0.1.4 #828 to v0.2.x #831

Merged
merged 4 commits into from
Dec 10, 2018
Merged

Backport Pin controller-runtime to v0.1.4 #828 to v0.2.x #831

merged 4 commits into from
Dec 10, 2018

Conversation

madorn
Copy link
Member

@madorn madorn commented Dec 8, 2018

operator-sdk branch v0.2.x currently broken due to recent controller-runtime bump to v0.1.8. See #828. Backport @lilic's master fix to v0.2.x.

lilic added 2 commits December 8, 2018 09:41
@lilic @madorn
pkg/scaffold: Adjust gopkgtoml test to reflect change
0feec71
@lilic @madorn
pkg/scaffold: Pin controller-runtime to exactly v0.1.4
3a31221 
When you specify a version without an operator, dep automatically uses
the ^ operator by default. That means it will take the major range.
That broke building of generated operators due to missing dependencies
in controller-runtime latest release.
@openshift-ci-robot openshift-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Dec 8, 2018
@hasbro17
Copy link
Contributor

@shawn-hurley @mhrivnak Any clue why the ansible based memcached-operator is giving the following error:

2018-12-10T05:19:52.612Z	ERROR	runner	runner/runner.go:250	error from ansible-runner	{"job": "8716441061609681071", "name": "example-memcached", "namespace": "default", "error": "exit status 1"}

I guess we've fixed some things on the master since, so it might be related to the fixes from #819

And since we added #812 later as well the logs above aren't too clear on why the runner failed.

@madorn Can you please cherrypick the commit from #812 so we can see why the ansible runner is failing to run.

@mhrivnak @madorn
pkg/ansible/runner: logs the stdout and stderr from ansible-runner if…
07138af 
… the command exits non-zero

This is helpful in a case where ansible-runner itself is broken, such as when a
python library it depends on gets installed at an incompatible version (a
problem we've encountered multile times).
@shawn-hurley
Copy link
Member

@madorn you will probably need the dockerfile fixes? #813

@estroz
Copy link
Member

estroz commented Dec 10, 2018

/hold
@madorn what errors are you seeing? all tests passed in CI and locally (for me).

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 10, 2018
@madorn
Copy link
Member Author

madorn commented Dec 10, 2018

@estroz Issue documented in #827 and fixed on master (#828).

operator-sdk's v0.2.x branch Gopkg.toml is not currently pinned to controller-runtime v0.1.4. It's automatically bumping to v0.1.8 causing incompatibility with apimachinery version kubernetes-1.11.2

cd $GOPATH/src/github.com/example-inc/
operator-sdk new app-operator
cd app-operator
operator-sdk add api --api-version=app.example.com/v1alpha1 --kind=AppService
operator-sdk add controller --api-version=app.example.com/v1alpha1 --kind=AppService
operator-sdk up local (or build)
[...]
# github.com/example-inc/app-operator/vendor/sigs.k8s.io/controller-runtime/pkg/client
vendor/sigs.k8s.io/controller-runtime/pkg/client/unstructured_client.go:49:24: undefined: v1.CreateOptions
vendor/sigs.k8s.io/controller-runtime/pkg/client/unstructured_client.go:67:24: undefined: v1.UpdateOptions
vendor/sigs.k8s.io/controller-runtime/pkg/client/unstructured_client.go:145:26: too many arguments in call to r.UpdateStatus
vendor/sigs.k8s.io/controller-runtime/pkg/client/unstructured_client.go:145:30: undefined: v1.UpdateOptions```

@estroz
Copy link
Member

estroz commented Dec 10, 2018

/unhold
Missed which branch this was being merged into. Thanks for clarifying.

@openshift-ci-robot openshift-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 10, 2018
@AlexNPavel
Copy link
Contributor

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 10, 2018
AlexNPavel
AlexNPavel approved these changes Dec 10, 2018
View reviewed changes
Copy link
Contributor

@AlexNPavel AlexNPavel left a comment

Choose a reason for hiding this comment

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

LGTM

shawn-hurley
shawn-hurley approved these changes Dec 10, 2018
View reviewed changes
Copy link
Member

@shawn-hurley shawn-hurley left a comment

Choose a reason for hiding this comment

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

LGTM

hasbro17
hasbro17 approved these changes Dec 10, 2018
View reviewed changes
Copy link
Contributor

@hasbro17 hasbro17 left a comment

Choose a reason for hiding this comment

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

LGTM

@hasbro17 hasbro17 merged commit 1a5768c into operator-framework:v0.2.x Dec 10, 2018
@AlexNPavel AlexNPavel mentioned this pull request Dec 10, 2018
Merged
@hasbro17
Copy link
Contributor

@madorn Thanks for the fix. We've cut a patch release for v0.2.1 that should have this fix.
https://github.com/operator-framework/operator-sdk/releases/tag/v0.2.1

@h7kanna
Copy link

h7kanna commented Dec 11, 2018

I just tried this. Looks like still broken.
Gopkg.toml is created with

[[override]]
name = "sigs.k8s.io/controller-runtime"
version = "v0.1.4"

@AlexNPavel
Copy link
Contributor

@h7kanna Are you sure you're using v0.2.1 (operator-sdk --version)? I just tried both building from the git tag and using the provided binaries and both output the correct Gopkg.toml files.

@h7kanna
Copy link

h7kanna commented Dec 11, 2018

@AlexNPavel Nevermind.
My profile did not pick the update version (v0.2.1).
It's working.
Thanks.

@hasbro17 hasbro17 mentioned this pull request Dec 13, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shawn-hurley shawn-hurley shawn-hurley approved these changes

@AlexNPavel AlexNPavel AlexNPavel approved these changes

@hasbro17 hasbro17 hasbro17 approved these changes

Assignees
No one assigned
Labels
size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@madorn @hasbro17 @shawn-hurley @estroz @AlexNPavel @h7kanna @openshift-ci-robot @lilic @mhrivnak @joelanford