Skip to content

Conversation

@wking
Copy link
Member

@wking wking commented Sep 23, 2021

Following up on #821 with the changes made in order to land openshift/api#1011.

@openshift-ci openshift-ci bot requested review from jwmatthews and squeed September 23, 2021 18:32
…anded

Following up on openshift#821 with the changes made in order to land
openshift/api#1011.

The graph-data properties remain optional, because graph-data has been
used for unconditional blocks that do not declare risks today.  The
update service will continue to read those blocks, but will implement
them by pruning 'edges'.  So things that end up in 'conditionalEdges'
will still need all the metadata populated to keep David happy ;).
@wking wking force-pushed the targeted-edge-blocking-follow-up branch from 11a0be8 to 6af8d09 Compare September 23, 2021 20:52
wking added a commit to wking/cincinnati-graph-data that referenced this pull request Sep 23, 2021
So we can explain why we're blocking the different edges [1] (the
promql -> PromQL type change is in flight with [2]).

The zz in the filename for the vSphere hostname block ensures that one
sorts last alphabetically, because it's the broadest block, and legacy
Cincinnati services will prefer the final regular expression they load
for a given 'to' target.

This is basically a second attempt at my earlier 39bc2fb
(blocked-edges/4.7.4*: Targeted edge blocking and version 1.1.0,
2021-09-01, openshift#1056), which ended up getting reverted in da1254a
(Revert "blocked-edges/4.7.4*: Targeted edge blocking and version
1.1.0", 2021-09-21, openshift#1078) because the production Cincinnati was mad
about the 1.1.0 version string.  [3] taught Cincinnati to relax, and
now that's live (and we never shipped any versions that would be mad
about 1.1.0 to customers, the 4.6.0 Update Service operator went out
before [4]).

[1]: openshift/enhancements#821
[2]: openshift/enhancements#910
[3]: openshift/cincinnati#538
[4]: openshift/cincinnati#314
wking added a commit to wking/cincinnati-graph-data that referenced this pull request Sep 23, 2021
To keep us from forgetting to set 'url', etc. [1,2].

We'll probably want to have PromQL parser validation later [3], but I
write Python a lot faster than I write Go.  This should avoid obvious
mistakes while the PromQL parser gets wired up to a validator.

[1]: openshift/enhancements#821
[2]: openshift/enhancements#910
[3]: https://github.com/openshift/enhancements/blob/4412b51182ba6e007f1308a73e290e81fd66d092/enhancements/update/targeted-update-edge-blocking.md#promql-validation
@sdodson
Copy link
Member

sdodson commented Sep 24, 2021

/lgtm
/approve
Thanks for updating this.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Sep 24, 2021
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Sep 24, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sdodson

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 24, 2021
@openshift-merge-robot openshift-merge-robot merged commit 2cc2d9b into openshift:master Sep 24, 2021
@wking wking deleted the targeted-edge-blocking-follow-up branch September 24, 2021 19:13
wking added a commit to wking/cincinnati-graph-data that referenced this pull request Sep 27, 2021
So we can explain why we're blocking the different edges [1] (the
promql -> PromQL type change is in flight with [2]).

The zz in the filename for the vSphere hostname block ensures that one
sorts last alphabetically, because it's the broadest block, and legacy
Cincinnati services will prefer the final regular expression they load
for a given 'to' target.

This is basically a second attempt at my earlier 39bc2fb
(blocked-edges/4.7.4*: Targeted edge blocking and version 1.1.0,
2021-09-01, openshift#1056), which ended up getting reverted in da1254a
(Revert "blocked-edges/4.7.4*: Targeted edge blocking and version
1.1.0", 2021-09-21, openshift#1078) because the production Cincinnati was mad
about the 1.1.0 version string.  [3] taught Cincinnati to relax, and
now that's live (and we never shipped any versions that would be mad
about 1.1.0 to customers, the 4.6.0 Update Service operator went out
before [4]).

[1]: openshift/enhancements#821
[2]: openshift/enhancements#910
[3]: openshift/cincinnati#538
[4]: openshift/cincinnati#314
wking added a commit to wking/cincinnati-graph-data that referenced this pull request Sep 27, 2021
So we can explain why we're blocking the different edges [1] (the
promql -> PromQL type change is in flight with [2]).

The zz in the filename for the vSphere hostname block ensures that one
sorts last alphabetically, because it's the broadest block, and legacy
Cincinnati services will prefer the final regular expression they load
for a given 'to' target.

This is basically a second attempt at my earlier 39bc2fb
(blocked-edges/4.7.4*: Targeted edge blocking and version 1.1.0,
2021-09-01, openshift#1056), which ended up getting reverted in da1254a
(Revert "blocked-edges/4.7.4*: Targeted edge blocking and version
1.1.0", 2021-09-21, openshift#1078) because the production Cincinnati was mad
about the 1.1.0 version string.  [3] taught Cincinnati to relax, and
now that's live (and we never shipped any versions that would be mad
about 1.1.0 to customers, the 4.6.0 Update Service operator went out
before [4]).

[1]: openshift/enhancements#821
[2]: openshift/enhancements#910
[3]: openshift/cincinnati#538
[4]: openshift/cincinnati#314
wking added a commit to wking/openshift-enhancements that referenced this pull request Sep 7, 2022
…id typo

It snuck in with 6af8d09
(enhancements/update/targeted-update-edge-blocking: Update after API
landed, 2021-09-20, openshift#910).
wking added a commit to wking/openshift-enhancements that referenced this pull request Jun 7, 2023
Following up on 6af8d09
(enhancements/update/targeted-update-edge-blocking: Update after API
landed, 2021-09-20, openshift#910), with more changes to align this enhancement
with the actual implementation.

We never implemented Evaluating, because Recommended=Unknown means
we're trying (and failing) to figure out if some of the target's risks
apply to the cluster.

And:

  // +kubebuilder:validation:Required
  // +kubebuilder:validation:MinItems=1
  // +listType=atomic
  // +required
  MatchingRules []ClusterCondition `json:"matchingRules"`

means that empty matchingRules is not possible, as previously discussed around:

  Mitigating that "where did that edge go?" confusion in the
  required-property alternative, David expects graph-admins to be
  keeping watch over enough ClusterVersion content...

in the alternatives section of the enhancement.
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.

3 participants