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
OCPBUGS-14561: Prevent ci/prow/versions from failing on PR against release-xxx #1969
Conversation
@machine424: This pull request references MON-3173 which is a valid jira issue. 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. |
@@ -42,14 +42,6 @@ for c in $COMPONENTS; do | |||
REMOTE="$(version_from_remote "$SLUG")" | |||
REMOTE="${REMOTE#v}" | |||
|
|||
if [ "$REMOTE" = "" ] && [ "$INTERACTIVE" != "true" ]; then |
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.
EAFP, I don't think INTERACTIVE=true is used. This was a copy/paste from upstream.
now, we'll set the version to empty string if not able to fetch it, I think it's better than just using local versions.
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 never used INTERACTIVE
afair.
@machine424: This pull request references MON-3173 which is a valid jira issue. 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. |
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.
/lgtm
/retest |
…implify. Make hack/generate-versions.sh do nothing on PR that are not against the main branch as the components versions are only updated on the main branch (on release-xx branches, components kepp their versions when fixed) Add a 'make check-versions' target to run the versions check. CI jobs will be updated to use this.
I added a commit to synchronize the versions
|
kubeStateMetrics: 2.8.2 | ||
nodeExporter: 1.5.0 | ||
promLabelProxy: 0.6.0 | ||
prometheus: 2.43.0 | ||
prometheusAdapter: 0.10.0 | ||
prometheusOperator: 0.63.0 | ||
thanos: 0.31.0 | ||
thanos: 0.30.2 |
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.
Could you please help me understand why addressing https://issues.redhat.com//browse/MON-3173 require versions to be changed?
If it isn't strictly needed, we shouldn't introduce changes to the product in a PR that addresses a development workflow tweak.
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 explained this in the separate commit where I make the change: the versions job was failing because these versions were not aligned with the ones that are already used in the product.
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.
While I agree a separate PR would be ideal, I'd be happy to merge this since its in its own commit and clearly states why it was changed.
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.
If #1924 gets merged before this one, I'd discard my commit.
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.
While I agree a separate PR would be ideal, I'd be happy to merge this since its in its own commit and clearly states why it was changed.
I am not against merging this but .. I wanted to understand why the versions are being downgraded. You might have more context than I do since the commit doesn't provide much information to me. It seems evident from the code that the versions are being downgraded but the message could be clearer about the reason behind this change.
E.g. Is it because the synbot disabled?
Synchronize version of thanos as it was downgraded and the sync bot is disabled
Synchronize version of kubeRbacProxy
the versions job was failing because these versions were not aligned with the ones that are already used in the product
If it isn't too much to ask for could you please incorporate that the message in the commit to indicate this. E.g.
Fix versions ci job
`versions` ci job is failing because of versions of thanos and kube-rbac-proxy
are different to the ones that are already used in the product. This patch fixes
that by setting the correct version of thanos and kube-rbac-proxy.
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 adjusted the message (spent more time doing so than making the change itself :))
Maybe we should doc the versions job somewhere, otherwise we'll need to explain what it does on every commit, which just duplicates info which can become obsolete and that we can never adjust thus misleading anyone who may read this in 6 months.
I think sometimes it's better to have this sort of knowledge documented only once on a medium we can keep up to date. This way we can even use the same commit message than the bot https://github.com/openshift/cluster-monitoring-operator/pull/1924/commits in case we want to merge before it.
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.
👍 for better docs, I have something in the pipeline for rhobs/handbook.
i'd argue for merging this. We need a bug to move this anyway and #1924 would need yet another one. So lets combine them and skip the additional Jira interaction.
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 can help with rhobs/handbook, it falls within https://issues.redhat.com/browse/MON-3217
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.
+1 on merging this. The question wasn't intended to block the merge.
/lgtm |
The versions in jsonnet/versions.yaml are only used as metadata (added as Kube labels) CMO is already using the appropriate versions from downstream. The thanos downgrade is justified, see openshift/thanos#112 for more details. Signed-off-by: Ayoub Mrini <amrini@redhat.com>
/retest-required |
/label docs-approved |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jan--f, machine424 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 |
/retitle OCPBUGS-14561: Prevent ci/prow/versions from failing on PR against release-xxx |
@machine424: This pull request references Jira Issue OCPBUGS-14561, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
/jira refresh |
@machine424: This pull request references Jira Issue OCPBUGS-14561, which is invalid:
Comment 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. |
/jira refresh |
@machine424: This pull request references Jira Issue OCPBUGS-14561, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
@machine424: all tests passed! Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
@machine424: Jira Issue OCPBUGS-14561: Some pull requests linked via external trackers have merged: The following pull requests linked via external trackers have not merged:
These pull request must merge or be unlinked from the Jira bug in order for it to move to the next state. Once unlinked, request a bug refresh with Jira Issue OCPBUGS-14561 has not been moved to the MODIFIED state. 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. |
…onization (#39773) Using tht target makes it easier to run test locally and it makes logs clearer (git diff --exit-code didn't appear in logs before). The target is introduced in openshift/cluster-monitoring-operator#1969
…onization (openshift#39773) Using tht target makes it easier to run test locally and it makes logs clearer (git diff --exit-code didn't appear in logs before). The target is introduced in openshift/cluster-monitoring-operator#1969
…implify.
Make hack/generate-versions.sh do nothing on PR that are not against the main branch as the components versions are only updated on the main branch (on release-xx branches, components kepp their versions when fixed)
Add a 'make check-versions' target to run the versions check. CI jobs will be updated to use this.