-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Machine api operator #1095
Machine api operator #1095
Conversation
e73a5c2
to
81f7a71
Compare
@bbguimaraes could you take a look at this? |
cluster/ci/config/prow/config.yaml
Outdated
@@ -4179,6 +4182,35 @@ postsubmits: | |||
- --target=[images] | |||
- --promote | |||
|
|||
openshift/machine-api-operator: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a PR job that also builds the images, that way we know that it works (the problem with branch only jobs is that if they fail they fail silently)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this should also build images, I changed name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go ahead and add the PR job that builds the images as well (so put it in presubmits, copy one of the existing ones that has --target=[images] --target=unit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added another job called pull-ci-openshift-machine-api-operator-unit
and based on pull-ci-kubernetes-coredns-unit
.
cluster/ci/config/prow/config.yaml
Outdated
labels: | ||
- lgtm | ||
missingLabels: | ||
- needs-ok-to-test | ||
- do-not-merge/work-in-progress | ||
- do-not-merge/hold | ||
merge_method: | ||
openshift/machine-api-operator: squash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you know if this has been tested with batch merges? I'm not familiar with this and I know most prow users don't use it, so I don't want you to hit surprising and magic bugs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thi is used by upstream, but I will drop it from here.
"requests": { "cpu": "100m", "memory": "200Mi" }, | ||
"limits": { "cpu": "2", "memory": "4Gi" } | ||
}, | ||
"bin": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need this much CPU, you can probably drop both bin and unit for now unless you have data otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that was a copy paste. Reduced.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant remove the bin and unit sections - the "*" section already provides defaults, that way you don't have to do it per container.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I misunderstood you. Fixed.
6a29cb7
to
650689b
Compare
73ed376
to
f946d7b
Compare
cluster/ci/config/prow/config.yaml
Outdated
- --target=[images] | ||
- --promote | ||
|
||
- name: pull-ci-openshift-machine-api-operator-unit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be under "presubmits" (with other PR jobs) and the branch job should be under "postsubmits". The section you are in determines when you are called.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved
05f661c
to
e306002
Compare
bump! how are things looking on this @smarterclayton (or possibly @stevekuznetsov ) |
e306002
to
2565066
Compare
I have migrated presubmits and postsubmits configuration from @stevekuznetsov @smarterclayton could you review this? |
/lgtm |
@paulfantom: Updated the
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Implement simple test and build pipeline for machine-api-operator repository. To stay as close as possible to developer workflow,
make
command is used to build binary.@smarterclayton this will also need a place to store images? Can I expect new repository in deocker hub or quay to be set up?