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.

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

sig-cl: resolve some warnings with the maintainers tool + governance updates #6402

Conversation

neolit123
Copy link
Member

@neolit123 neolit123 commented Jan 26, 2022

list of commits:

sig-cl: update CONTRIBUTING.md

- Fix malformed link
- Include link to VODs
sig-cl: remove the kubespray Zoom meeting

As discussed with the kubespray maintainers:
https://kubernetes.slack.com/archives/C2V9WJSJD/p1643716538243509[](https://github.com/neolit123)

Their Zoom meeting is no longer active.
The maintainers expressed that Slack is sufficient for them
and we should remove the meeting from the calendar for the time being.

Remove the meeting from sigs.yaml
It was also removed from the SIG CL calendar.
sig-cl: resolve some warnings with the maintainers tool

- Remove kube-up as a subproject. The tool correctly discovers that
we do not have OWNERS under k/k/cluster.
- Add mailing list / slack / description to projects that did not
have it.
- Rename kops -> kOps to match the project rename that happened
some time ago.
- Add more OWNERS URLs in the kubeadm subproject section. These are
not kubeadm specific repositories, but kubeadm is the best
fit given the top level SIG struct does not have OWNERS.

list of ERRORS from the maintainers tool:

~/go/bin/maintainers --kubernetes-directory=/home/lubo/go/src/k8s.io/kubernetes audit sig-cluster-lifecycle | grep ERROR
ERROR: unable to parse owners file at https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-provider-aws/master/OWNERS url - error unmarshaling JSON: while decoding JSON: json: unknown field "emeritus_maintainers"
ERROR: unable to parse owners file at https://raw.githubusercontent.com/kubernetes-sigs/cluster-api-provider-openstack/master/OWNERS url - error unmarshaling JSON: while decoding JSON: json: unknown field "emeritus_maintainers"
ERROR: unable to read file /home/lubo/go/src/k8s.io/kubernetes/_output/local/go/pkg/mod/k8s.io/kube-openapi@v0.0.0-20210421082810-95288971da7e/OWNERS - error unmarshaling JSON: while decoding JSON: json: unknown field "emritus_approvers"

list of warnings BEFORE this change:

``` WARNING: missing 'description' key WARNING: missing 'mailing_list' in contact WARNING: missing 'description' key WARNING: missing 'mailing_list' in contact WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'meetings' key WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'meetings' key WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'meetings' key WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'meetings' key WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'meetings' key WARNING: missing 'description' key WARNING: missing 'mailing_list' in contact WARNING: missing 'description' key WARNING: missing 'mailing_list' in contact WARNING: missing 'description' key WARNING: missing 'mailing_list' in contact WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: missing 'description' key WARNING: missing 'contact' key WARNING: needs labels reflecting sig-cluster-lifecycle - https://raw.githubusercontent.com/kubernetes/kubernetes/master/cluster/OWNERS WARNING: needs an alias as approver/reviewer reflecting sig-cluster-lifecycle - https://raw.githubusercontent.com/kubernetes/kubernetes/master/cluster/OWNERS WARNING: missing 'meetings' key WARNING: missing 'description' key WARNING: missing 'mailing_list' in contact WARNING: needs an alias as approver/reviewer reflecting sig-cluster-lifecycle - https://raw.githubusercontent.com/kubernetes/kubernetes/master/cmd/kubeadm/OWNERS WARNING: missing 'description' key WARNING: missing 'mailing_list' in contact WARNING: missing 'description' key WARNING: missing 'mailing_list' in contact WARNING: file staging/src/k8s.io/cluster-bootstrap/OWNERS should be in one of ["sig-cluster-lifecycle"] based on labels/aliases WARNING: file staging/src/k8s.io/component-base/configz/OWNERS should be in one of ["sig-cluster-lifecycle"] based on labels/aliases WARNING: file staging/src/k8s.io/component-base/featuregate/OWNERS should be in one of ["sig-api-machinery" "sig-cluster-lifecycle"] based on labels/aliases WARNING: file test/e2e/lifecycle/OWNERS should be in one of ["sig-cluster-lifecycle"] based on labels/aliases WARNING: file test/e2e_kubeadm/OWNERS should be in one of ["sig-cluster-lifecycle"] based on labels/aliases ```

list of WARNINGS after this change:

WARNING: needs an alias as approver/reviewer reflecting sig-cluster-lifecycle - https://raw.githubusercontent.com/kubernetes/kubernetes/master/cmd/kubeadm/OWNERS
WARNING: needs an alias as approver/reviewer reflecting sig-cluster-lifecycle - https://raw.githubusercontent.com/kubernetes/kubernetes/master/test/e2e/lifecycle/OWNERS
WARNING: needs an alias as approver/reviewer reflecting sig-cluster-lifecycle - https://raw.githubusercontent.com/kubernetes/kubernetes/master/test/e2e_kubeadm/OWNERS
WARNING: needs an alias as approver/reviewer reflecting sig-cluster-lifecycle - https://raw.githubusercontent.com/kubernetes/kubernetes/master/staging/src/k8s.io/component-base/configz/OWNERS
WARNING: needs an alias as approver/reviewer reflecting sig-cluster-lifecycle - https://raw.githubusercontent.com/kubernetes/kubernetes/master/staging/src/k8s.io/component-base/featuregate/OWNERS

these seem to be due to missing SIG-CL in OWNER_ALIAS files (?) and maybe should be optional.

@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jan 26, 2022
@neolit123
Copy link
Member Author

/kind cleanup
/sig cluster-lifecycle
/priority backlog
/triage accepted
/hold
for comments

@k8s-ci-robot k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. priority/backlog Higher priority than priority/awaiting-more-evidence. triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Jan 26, 2022
@neolit123
Copy link
Member Author

neolit123 commented Jan 26, 2022

Q: would it make sense to have omitempty in the YAML spec for meetings?

with the following patch:

diff --git a/generator/app.go b/generator/app.go
index bae4903e..fbd8ee8b 100644
--- a/generator/app.go
+++ b/generator/app.go
@@ -84,10 +84,10 @@ type Person struct {
 // Meeting represents a regular meeting for a group.
 type Meeting struct {
        Description   string
-       Day           string
-       Time          string
-       TZ            string
-       Frequency     string
+       Day           string `yaml:",omitempty"`
+       Time          string `yaml:",omitempty"`
+       TZ            string `yaml:",omitempty"`
+       Frequency     string `yaml:",omitempty"`
        URL           string `yaml:",omitempty"`
        ArchiveURL    string `yaml:"archive_url,omitempty"`
        RecordingsURL string `yaml:"recordings_url,omitempty"`
diff --git a/generator/sig_readme.tmpl b/generator/sig_readme.tmpl
index 4bb61997..54f8c60f 100644
--- a/generator/sig_readme.tmpl
+++ b/generator/sig_readme.tmpl
@@ -92,7 +92,7 @@ The following [subprojects][subproject-definition] are owned by sig-{{.Label}}:
   - [Mailing List]({{.Contact.MailingList}})
 {{- end }}
 {{- if .Contact.GithubTeams }}
-  - GitHub Teams: 
+  - GitHub Teams:
 {{- range .Contact.GithubTeams }}
     - [@kubernetes/{{.Name}}](https://github.com/orgs/kubernetes/teams/{{.Name}}) {{- if .Description }} - {{.Description}}{{- end}}
 {{- end }}
@@ -101,7 +101,7 @@ The following [subprojects][subproject-definition] are owned by sig-{{.Label}}:
 {{- if .Meetings }}
 - **Meetings:**
 {{- range .Meetings }}
-  - {{.Description}}: [{{.Day}}s at {{.Time}} {{.TZ}}]({{.URL}}) ({{.Frequency}}). [Convert to your timezone](http://www.thetimezoneconverter.com/?t={{.Time}}&tz={{.TZ | tzUrlEncode}}).
+  - {{.Description}}{{- if .Day }}: [{{.Day}}s at {{.Time}} {{.TZ}}]({{.URL}}) ({{.Frequency}}). [Convert to your timezone](http://www.thetimezoneconverter.com/?t={{.Time}}&tz={{.TZ | tzUrlEncode}}).{{- end}}
 {{- if .ArchiveURL }}
     - [Meeting notes and Agenda]({{.ArchiveURL}}).
 {{- end }}

it allows us to do things like:

     meetings:
     - description: Use the main Cluster API meeting
-      day: ""
-      time: ""
-      tz: ""
-      frequency: ""

this would tell the user, "yes, there is a meeting, but not a specific one for this subproject and you can go to another meeting".

WDYT?

EDIT: included as first commit here.

@neolit123 neolit123 force-pushed the 1.24-sig-cl-resolve-warnings-with-maintainers-tool branch from 8e8e47e to 7fb3493 Compare January 26, 2022 15:09
@k8s-ci-robot k8s-ci-robot added the sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. label Jan 26, 2022
@neolit123 neolit123 force-pushed the 1.24-sig-cl-resolve-warnings-with-maintainers-tool branch 2 times, most recently from 8f0a177 to 871608a Compare January 26, 2022 15:22
@neolit123
Copy link
Member Author

/cc @fabriziopandini

@fabriziopandini
Copy link
Member

fabriziopandini commented Jan 26, 2022

/lgtm
for the changes to sigs.yaml
also, I notice that cluster-api-operator is missing; might be worth cross-checking is also if someone else is missing and then open issues in their repositories

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2022
@neolit123
Copy link
Member Author

also, I notice that cluster-api-operator is missing; might be worth cross-checking is also if someone else is missing and then open issues in their repositories

yep, some could be missing. they can be added in a follow up.
as part of the repository / subproject creations we should tell the elected maintainers to PR the new subproject into sigs.yaml and we can LGTM the PR.

@alexander-demicev
Copy link
Member

alexander-demicev commented Jan 27, 2022

I requested slack channel for CAPI operator #6404, I'll update the PR #6304 when it's ready.

@fabriziopandini
Copy link
Member

@neolit123 I have checked all the CAP* providers and all of them are listed except Cluster API operator which will be fixed by the PR linked above. Nevertheless, I have asked the various team to update sub-project info, given that in some cases they are incomplete (e.g lack of slack channel, etc.)

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 3, 2022
@neolit123 neolit123 force-pushed the 1.24-sig-cl-resolve-warnings-with-maintainers-tool branch from 871608a to 7c3bf71 Compare February 8, 2022 18:24
@k8s-ci-robot k8s-ci-robot removed lgtm "Looks good to me", 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 Feb 8, 2022
@dims
Copy link
Member

dims commented Feb 9, 2022

/assign @mrbobbytables
/unassign

@neolit123 looks good in general, want to let sig-contribex folks look at the changes proposed in the tools

@k8s-ci-robot k8s-ci-robot assigned mrbobbytables and unassigned dims Feb 9, 2022
@neolit123 neolit123 changed the title sig-cluster-lifecycle: resolve some warnings with the maintainers tool sig-cl: resolve some warnings with the maintainers tool + governance updates Feb 14, 2022
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 21, 2022
@mrbobbytables
Copy link
Member

looping in TLs 👍
/assign @cblecker @nikhita

@neolit123 neolit123 force-pushed the 1.24-sig-cl-resolve-warnings-with-maintainers-tool branch from 6f868d4 to fd511e9 Compare February 22, 2022 16:54
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 22, 2022
@neolit123
Copy link
Member Author

neolit123 commented Feb 22, 2022

i'm considering dropping the first commit here - generator: allow only description under subproject meetings...and leaving some subprojects without a zoom call info until further notice.
the maintainers tool currently complains (warning level) if a subproject does not have a meetings struct, but AFAIK there is no clear requirement for subprojects to have zoom calls.

e.g. kubespray expressed that they don't want to have a zoom call and slack / github is enough for them.

@neolit123 neolit123 force-pushed the 1.24-sig-cl-resolve-warnings-with-maintainers-tool branch from fd511e9 to 3a8787e Compare February 23, 2022 16:26
@neolit123
Copy link
Member Author

neolit123 commented Feb 23, 2022

dropped the first commit and left some subprojects without meetings descriptions (as they were before).
the PR no longer touches the generator. PTAL for approval.

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 8, 2022
- Fix malformed link
- Include link to VODs
As discussed with the kubespray maintainers:
https://kubernetes.slack.com/archives/C2V9WJSJD/p1643716538243509

Their Zoom meeting is no longer active.
The maintainers expressed that Slack is sufficient for them
and we should remove the meeting from the calendar for the time being.

Remove the meeting from sigs.yaml
It was also removed from the SIG CL calendar.
- Remove kube-up as a subproject. The tool correctly discovers that
we do not have OWNERS under k/k/cluster.
- Add mailing list / slack / description to projects that did not
have it.
- Rename kops -> kOps to match the project rename that happened
some time ago.
- Add more OWNERS URLs in the kubeadm subproject section. These are
not kubeadm specific repositories, but kubeadm is the best
fit given the top level SIG struct does not have OWNERS.
@neolit123 neolit123 force-pushed the 1.24-sig-cl-resolve-warnings-with-maintainers-tool branch from 3a8787e to 0df6fd0 Compare April 4, 2022 15:23
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 4, 2022
@mrbobbytables
Copy link
Member

/approve

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 4, 2022
@neolit123
Copy link
Member Author

/cc @dims
PTAL for LGTM

to unblock the SIG yearly report.

@k8s-ci-robot k8s-ci-robot requested a review from dims April 4, 2022 15:42
@dims
Copy link
Member

dims commented Apr 4, 2022

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 4, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dims, mrbobbytables, neolit123, vincepri

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 8e952ec into kubernetes:master Apr 4, 2022
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. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/contributor-experience Categorizes an issue or PR as relevant to SIG Contributor Experience. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. triage/accepted Indicates an issue or PR is ready to be actively worked on.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants