Skip to content

Conversation

brb
Copy link
Member

@brb brb commented Jun 19, 2020

This PR replaces svc master by svc frontend and slave by backend slot.

@brb brb added pending-review area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. kind/cleanup This includes no functional changes. area/daemon Impacts operation of the Cilium daemon. release-note/misc This PR makes changes that have no direct user impact. labels Jun 19, 2020
@brb brb requested a review from a team as a code owner June 19, 2020 14:31
@brb brb requested a review from a team June 19, 2020 14:31
@brb brb requested a review from a team as a code owner June 19, 2020 14:31
@brb brb force-pushed the pr/brb/lb-neutral-term branch from 4b9152f to bca81a0 Compare June 19, 2020 14:32
@brb
Copy link
Member Author

brb commented Jun 19, 2020

test-me-please

Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for that.

@brb brb force-pushed the pr/brb/lb-neutral-term branch from bca81a0 to 8a3deca Compare June 19, 2020 14:54
@brb
Copy link
Member Author

brb commented Jun 19, 2020

test-me-please

@coveralls
Copy link

coveralls commented Jun 19, 2020

Coverage Status

Coverage decreased (-0.02%) to 36.967% when pulling 46ee0eb on pr/brb/lb-neutral-term into c4b2c1e on master.

qmonnet added a commit to qmonnet/cilium that referenced this pull request Jun 30, 2020
Although there is no explicit coding style for the BPF code, several
developers tend to follow the kernel coding style conventions [0]. To
try and maintain a consistent style all over the code, this commit adds
kernel's checkpatch.pl to the list of checks performed on the source as
part as BPF's "make check".

Some details on the check:

- We only run checkpatch.pl on the bpf/ directory.

- A new "checkpatch" BPF make target is created, and added as a
  dependency to "check".

- However, we only run it on the latest changes by default, meaning the
  changes since newest Git parent reference plus the changes returned by
  "git diff HEAD".

- For manual checks, the bash script used to launch checkpatch has an
  option to run the checks on all the bpf/ source code, independently of
  the Git history.

- GitHub action runs checkpatch.pl on all commits from the PR. There is
  no mechanism to bypass checkpatch errors, Janitors will have to decide
  if issues reported by checkpatch should be ignored.

The checkpatch.pl and spelling.txt files (both under GPLv2) are directly
copied from the kernel repository, as per commit f436a58e2619
("checkpatch: fix CONST_STRUCT when const_structs.checkpatch is
missing") (in linux-next as of the creation of this patch). File
deprecated_terms.txt is created from this PR, and populated in prevision
of the changes proposed in cilium#12206. The script checkpatch.sh (bash
script) launches checkpatch.pl with the relevant arguments and options.
COPYING file is copied from bpf/COPYING, to make sure people get a
version of the license boilerplate with the checkpatch code.

[0] https://www.kernel.org/doc/html/v5.6/process/coding-style.html

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
aanm pushed a commit that referenced this pull request Jul 1, 2020
Although there is no explicit coding style for the BPF code, several
developers tend to follow the kernel coding style conventions [0]. To
try and maintain a consistent style all over the code, this commit adds
kernel's checkpatch.pl to the list of checks performed on the source as
part as BPF's "make check".

Some details on the check:

- We only run checkpatch.pl on the bpf/ directory.

- A new "checkpatch" BPF make target is created, and added as a
  dependency to "check".

- However, we only run it on the latest changes by default, meaning the
  changes since newest Git parent reference plus the changes returned by
  "git diff HEAD".

- For manual checks, the bash script used to launch checkpatch has an
  option to run the checks on all the bpf/ source code, independently of
  the Git history.

- GitHub action runs checkpatch.pl on all commits from the PR. There is
  no mechanism to bypass checkpatch errors, Janitors will have to decide
  if issues reported by checkpatch should be ignored.

The checkpatch.pl and spelling.txt files (both under GPLv2) are directly
copied from the kernel repository, as per commit f436a58e2619
("checkpatch: fix CONST_STRUCT when const_structs.checkpatch is
missing") (in linux-next as of the creation of this patch). File
deprecated_terms.txt is created from this PR, and populated in prevision
of the changes proposed in #12206. The script checkpatch.sh (bash
script) launches checkpatch.pl with the relevant arguments and options.
COPYING file is copied from bpf/COPYING, to make sure people get a
version of the license boilerplate with the checkpatch code.

[0] https://www.kernel.org/doc/html/v5.6/process/coding-style.html

Signed-off-by: Quentin Monnet <quentin@isovalent.com>
@brb brb force-pushed the pr/brb/lb-neutral-term branch from 8a3deca to e35c7e1 Compare July 13, 2020 13:33
@brb brb requested review from aanm, joestringer and borkmann July 13, 2020 13:34
@brb brb force-pushed the pr/brb/lb-neutral-term branch from e35c7e1 to 0be845b Compare July 13, 2020 13:36
@brb
Copy link
Member Author

brb commented Jul 13, 2020

test-me-please

@brb
Copy link
Member Author

brb commented Jul 13, 2020

The checkpatch.pl failure looks harmless:

lb: Use more neutral terminology
=========================================================
WARNING:DEPRECATED_TERM: Use of 'master' is deprecated, please 'frontend', instead.
#6: 
This commit replaces "svc master" by "svc frontend" and "slave" by

WARNING:DEPRECATED_TERM: Use of 'slave' is deprecated, please 'backend', instead.
#6: 
This commit replaces "svc master" by "svc frontend" and "slave" by


NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] lb: Use more neutral terminology" has style problems, please review.

NOTE: Ignored message types: C99_COMMENT_TOLERANCE COMPLEX_MACRO CONSTANT_CONVERSION CONST_STRUCT EMAIL_SUBJECT JIFFIES_COMPARISON LEADING_SPACE LONG_LINE_COMMENT MACRO_WITH_FLOW_CONTROL MULTISTATEMENT_MACRO_USE_DO_WHILE PRINTK_WITHOUT_KERN_LEVEL TRAILING_SEMICOLON TRAILING_STATEMENTS VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.
All done
make: *** [checkpatch] Error 1
Makefile.bpf:47: recipe for target 'checkpatch' failed
make: Leaving directory '/home/runner/work/cilium/cilium/bpf'
##[error]Process completed with exit code 2.

@brb brb force-pushed the pr/brb/lb-neutral-term branch from 0be845b to 46ee0eb Compare July 13, 2020 13:39
@brb
Copy link
Member Author

brb commented Jul 13, 2020

test-me-please

This commit replaces "svc master" by "svc frontend" and "slave" by
"backend slot".

Signed-off-by: Martynas Pumputis <m@lambda.lt>
@qmonnet qmonnet merged commit 308d67f into master Jul 15, 2020
@qmonnet qmonnet deleted the pr/brb/lb-neutral-term branch July 15, 2020 16:15
@borkmann
Copy link
Member

I've added the 1.8 backport flag to avoid merge conflicts from this cleanup with future fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/daemon Impacts operation of the Cilium daemon. area/datapath Impacts bpf/ or low-level forwarding details, including map management and monitor messages. kind/cleanup This includes no functional changes. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants