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

move to openshift/api types #212

Merged
merged 3 commits into from
Feb 1, 2019
Merged

move to openshift/api types #212

merged 3 commits into from
Feb 1, 2019

Conversation

deads2k
Copy link
Contributor

@deads2k deads2k commented Jan 21, 2019

switches us to use the new API type

@tnozicka is there anything I have to do the makefile?

@openshift-ci-robot openshift-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Jan 21, 2019
tnozicka
tnozicka reviewed Jan 21, 2019
View reviewed changes
Makefile Outdated
CODEGEN_OUTPUT_PACKAGE :=github.com/openshift/cluster-kube-apiserver-operator/pkg/generated
CODEGEN_API_PACKAGE :=github.com/openshift/cluster-kube-apiserver-operator/pkg/apis
CODEGEN_GROUPS_VERSION :=kubeapiserver:v1alpha1

# Include the library makefile
include $(addprefix ./vendor/github.com/openshift/library-go/alpha-build-machinery/make/, \
operator.mk \
Copy link
Contributor

@tnozicka tnozicka Jan 21, 2019 •
edited

Choose a reason for hiding this comment

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

@deads2k this is what includes the codegen package - those variables above are those that the package requires. at the beginning codegen seemed to be something used in all of our operators so it was included by default but we should revisit that choice once our 4.0 repos are setup based on usage.

to get rid of it you need to choose the targets manually like in https://github.com/openshift/cluster-config-operator/blob/929d69fff0ed1f31bc43276b582e957bbafd41b0/Makefile#L5-L10

@deads2k deads2k force-pushed the new-api branch 2 times, most recently from 46487e6 to f3ef06e Compare January 21, 2019 20:07
soltysh
soltysh approved these changes Jan 23, 2019
View reviewed changes
Copy link
Member

@soltysh soltysh left a comment

Choose a reason for hiding this comment

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

/lgtm
although you need to rebase

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2019
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2019
@deads2k deads2k added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2019
@deads2k
Copy link
Contributor Author

deads2k commented Jan 23, 2019

retagging

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2019
@deads2k deads2k added the lgtm Indicates that a PR is ready to be merged. label Jan 23, 2019
@openshift-bot
Copy link
Contributor

/retest

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

@deads2k
Copy link
Contributor Author

deads2k commented Jan 23, 2019 

                        "lastTransitionTime": "2019-01-23T17:38:19Z",
                        "message": "error syncing: timed out waiting for the condition during syncRequiredMachineConfigPools: error pool master is not ready. status: (total: 3, updated: 3, unavailable: 1)",
                        "status": "True",
                        "type": "Failing"

/retest

@openshift-bot
Copy link
Contributor

/retest

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

2 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-ci-robot openshift-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 24, 2019
@openshift-ci-robot openshift-ci-robot removed lgtm Indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 24, 2019
soltysh
soltysh approved these changes Jan 24, 2019
View reviewed changes
Copy link
Member

@soltysh soltysh 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 Jan 24, 2019
@openshift-ci-robot
Copy link

New changes are detected. LGTM label has been removed.

@deads2k deads2k added the lgtm Indicates that a PR is ready to be merged. label Jan 28, 2019
@deads2k
Copy link
Contributor Author

deads2k commented Jan 28, 2019

/retest

@openshift-bot
Copy link
Contributor

/retest

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

6 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@deads2k
Copy link
Contributor Author

deads2k commented Jan 29, 2019

found the issue. once openshift/cluster-authentication-operator#48 merges, this should pass

@openshift-bot
Copy link
Contributor

/retest

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

12 similar comments
@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@openshift-bot
Copy link
Contributor

/retest

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

@deads2k
Copy link
Contributor Author

deads2k commented Jan 31, 2019

/test all

@deads2k
Copy link
Contributor Author

deads2k commented Jan 31, 2019

/retest

4 similar comments
@deads2k
Copy link
Contributor Author

deads2k commented Jan 31, 2019

/retest

@deads2k
Copy link
Contributor Author

deads2k commented Jan 31, 2019

/retest

@deads2k
Copy link
Contributor Author

deads2k commented Jan 31, 2019

/retest

@sttts
Copy link
Contributor

sttts commented Feb 1, 2019

/retest

@openshift-merge-robot openshift-merge-robot merged commit 86b8202 into openshift:master Feb 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnozicka tnozicka tnozicka left review comments

@soltysh soltysh soltysh approved these changes

@adambkaplan adambkaplan Awaiting requested review from adambkaplan

@smarterclayton smarterclayton Awaiting requested review from smarterclayton

Assignees

@soltysh soltysh

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/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@deads2k @openshift-bot @openshift-ci-robot @sttts @soltysh @tnozicka @openshift-merge-robot