Please do not attempt to simplify this code

https://github.com/kubernetes/kubernetes/blob/60c4c2b2521fb454ce69dee737e3eb91a25e0535/pkg/controller/volume/persistentvolume/pv_controller.go

by whalesalad • 2 years ago

479 308 2 years ago

github.com

jftuga• 2 years ago

        // ==================================================================
        // PLEASE DO NOT ATTEMPT TO SIMPLIFY THIS CODE.
        // KEEP THE SPACE SHUTTLE FLYING.
        // ==================================================================
        //
        // This controller is intentionally written in a very verbose style. You will
        // notice:
        //
        // 1. Every 'if' statement has a matching 'else' (exception: simple error
        //    checks for a client API call)
        // 2. Things that may seem obvious are commented explicitly
        //
        // We call this style 'space shuttle style'. Space shuttle style is meant to
        // ensure that every branch and condition is considered and accounted for -
        // the same way code is written at NASA for applications like the space
        // shuttle.

wouldbecouldbe4• 2 years ago

hahaha well Kubernetes is the opposite of a special shuttle that keeps on flying. It crashes all the time, version updates etc. If you want stability go to apache or nginx.

Sohcahtoa822• 2 years ago

This comment is a complete non-sequitor. Kubernetes solves an entirely different problem from Apache and nginx.

andix3• 2 years ago

I've never seen Kubernetes crash. I don't have that much experience with operating k8s clusters, but on those I've seen it just kept on working.

OJFord2• 2 years ago

If you can solve your problem with nginx then yes you should be using nginx not kubernetes...

nimih• 2 years ago

If you think nginx is the right tool for solving problems like container deployment, service discovery, cluster scaling, and secret management, then I suppose it's not surprising that you think Kubernetes "crashes all the time" and that a 1 year rolling support window for software releases is an insurmountable obstacle. Kubernetes has a lot of genuine issues and rough edges, but you're kind of showing your ass when you make comments like this.

whalesalad11• 2 years ago

I tried to link directly to these lines, but hn's url sanitizer stripped off the anchor tags.

arwhatever• 2 years ago

When I looked into Go I found it a bit surprising that someone had created a non-expression-based language as late as ~2009.

I have not familiarized myself with the arguments against expression-based design but as a naive individual contributor/end-user-of-languages, expressions seem like one of the few software engineering decisions that doesn't actually "depend," but rather, designing languages around expressions seems to be unequivocally superior.

TurningCanadian• 2 years ago

instead of "expression" you meant "exception", right?

IAmGraydon• 2 years ago

I'm assuming you mean "non-exception". Apologies if I assume incorrectly.

In case I'm correct, this is from Andrew Gerrand, one of the creators of Go:

The reason we didn't include exceptions in Go is not because of expense. It's because exceptions thread an invisible second control flow through your programs making them less readable and harder to reason about.

In Go the code does what it says. The error is handled or it is not. You may find Go's error handling verbose, but a lot of programmers find this a great relief.

In short, we didn't include exceptions because we don't need them. Why add all that complexity for such contentious gains?

https://news.ycombinator.com/item?id=4159672

kuschku• 2 years ago

I used to be skeptical about introducing complex expressions to C-syntax languages for a long time until I saw how well Kotlin handled `when`.

Now every time I use typescript or go I have trouble trying to express what I want to say because `when` and similar expressions are just such a convenient way to think about a problem.

In go that means I usually end up extracting that code into a separate function with a single large `switch` statement with every case containing a `return` statement.

jiggawatts• 2 years ago

More to the point, the comment in the code mentions the “combinatorial” explosion of conditions that have to be carefully maintained by fallible meat brains.

In most modern languages something like this could be implemented using composition with interfaces or traits. Especially in Rust it’s possible to write very robust code that has identical performance to the if-else spaghetti, but is proven correct by the compiler.

I’m on mobile, so it’s hard to read through the code, but I noticed one section that tries to find an existing volume to use, and if it can’t, then it will provision one instead.

This could be three classes that implement the same interface:

    class ExistingVolume : IVolumeAllocator
    class CreateVolume : IVolumeAllocator
    class SeqVolumeAllocator : IVolumeAllocator

The last class takes a list of IVolumeAllocator abstract types as its input during construction and will try them in sequence. It could find and then allocate, or find in many different places before giving up and allocating, or allocating from different pools trying them in order.

Far more flexible and robust than carefully commented if-else statements!

Similarly, there's a number of "feature gate" if-else statements adding to the complexity. Let's say the CreateVolume class has two variants, the original 'v1' and an experimental 'v2' version. Then you could construct a SeqVolumeAllocator thus:

    allocator = new SeqVolumeAllocator( 
        featureFlag  ? new CreateVolumeV2() : new CreateVolumeV1(),
        new ExistingVolume() );

And then you never have to worry about the feature flag breaking control flow or error handling somewhere in a bizarre way.

See the legendary Andrei Alexandrescu demonstrating about a similar design in his CppCon talk “std::allocator is to allocation what std::vector is to vexation”: https://youtu.be/LIb3L4vKZ7U

supportengineer• 2 years ago

I went right into the code and looked for 'if' statements without 'else' statements. There are plenty. I don't see how you can have any exceptions to this rule if you are truly committed to capturing all branches.

mypalmike• 2 years ago

Someone has obviously simplified the code. Oops.

throwanem2• 2 years ago

If the 'if' condition matching always results in a thrown exception, a return, or likewise, then you don't really need an 'else' unless you're using a language which supports conditions and resumption (conformant Common Lisp implementations, and not really anything else I know of). The 'else', implicitly, is that the flow of control leaves the scope of the 'if' block at all.

(I haven't read far enough into the code to know that this is what they're doing, but the head matter I did read suggests as much. It's a common enough pattern, especially around eg argument validation and other sanity checks a function might perform to ensure it can do meaningful work at all.)

(I do wish HN supported an inline monospace markup, the <code> to a four-space indent's <pre>...)

OJFord2• 2 years ago

Yeah, I immediately scrolled down to see how silly I thought it looked, and.. it's not, there are plenty of ifs without elses. If you're going to have exceptions to that rule where it's 'simple' and not necessary, then congratulations you're using if (and else or not) just like everyone else?

jerlam• 2 years ago

// KEEP THE SPACE SHUTTLE FLYING.

I understand the intent, but it is a bit funny that the comment references a system that is no longer operational due to its poor safety record.

In ten years or so, will people even remember the Space Shuttle in a good light?

whalesalad11• 2 years ago

Well over 100 successful missions carrying a bunch of people and gear up into outer space and then bringing them back home.

I hold it in a good light now, and will likely continue to feel that way. As far as human progress and net good, it was a success.

joebob423• 2 years ago

100 is frankly not that impressive for software. A 2-9 system will sometimes work 100x in a row, and a 3-9 system usually will.

chgs• 2 years ago

100 success and 2 failures. About a 1.6% failure rate from memory.

That’s not a great record. Sure it’s a complex field, and it’s not as dangerous as say being US President, but a failure rate of >1% is not something to write home about.

ChrisMarshallNY3• 2 years ago

Wasn't a poor safety record whut killed the shuttle. It was the cost, and anticipation of degraded safety, in the future.

Even though more astronauts died, because of the two shuttle accidents, than any other NASA disaster, the safety record was absolutely amazing, when we consider everything that was going on.

The code seems damn good code.

bpodgursky2• 2 years ago

2/100 catastrophic failures (deaths of entire crew) is not a good record even by the standards of spaceflight.

andix3• 2 years ago

The space shuttle became obsolete technology after all those years. Would've needed a redesign.

lmm• 2 years ago

Even though more astronauts died, because of the two shuttle accidents, than any other NASA disaster, the safety record was absolutely amazing, when we consider everything that was going on.

That's "the operation was successful but the patient died" logic. Killing over 1% of your riders is not a good safety record! No ifs, no buts.

andix3• 2 years ago

This code is keeping the space shuttle flying in our memories. Kubernetes never kept the space shuttles flying in a literal way.

peoplefromibiza• 2 years ago

The shuttle program was killed by costs. Keeping it flying safely was really expensive.

Despite the safety record you mention, it was the best approximation of a spaceship that we, as a human species, ever created.

A spaceship program is something you either fully commit to by spending billions on it, or you abandon it.

Nevertheless, there has been nothing that has come close to it since.

evil-olive• 2 years ago

a system that is no longer operational due to its poor safety record

the safety problems with the shuttle were, broadly speaking, hardware problems and not software problems.

from "Appendix F - Personal Observations on Reliability of Shuttle" [0], which was Richard Feynman's appendix to the report on the 1986 Challenger disaster:

To summarize then, the computer software checking system and attitude is of the highest quality. There appears to be no process of gradually fooling oneself while degrading standards so characteristic of the Solid Rocket Booster or Space Shuttle Main Engine safety systems.

he specifically highlighted the quality of the avionics software as an example of how engineering on a project like the Shuttle could be done well, and wasn't doomed to be low-quality and unsafe simply by virtue of being a large complicated government project.

0: https://www.nasa.gov/history/rogersrep/v2appf.htm

trte9343r4• 2 years ago

Space Shuttle was part of "Star Wars" project to bring down the Soviet Union. It met its goals by being super expensive.

Space flights were just side project...

tomcam• 2 years ago

Is its poor safety record due to software failures?

seabass-labrax• 2 years ago

The situation with the Space Shuttle is more complex than simply poor safety. In terms of missions, it has a better record than many other launch vehicles - 2 fatal missions out of 135 for the shuttle, 2 out of 66 for the Soviet-era Soyuz, and a frighteningly poor 1 fatal mission out of only 12 spaceflights for SpaceShipTwo.

However, the Space Shuttle had a much larger crew capacity than most missions probably needed (up to eight astronauts compared to Apollo or Soyuz's three), especially considering that the majority of Soviet/Roscosmos, ESA and CNSA missions were autonomous and completely unmanned - no crew to endanger!

Perhaps that makes the metaphor even better for Kubernetes: an highly engineered, capable and multi-purpose system requiring considerable attention, and probably used a little more than it should be.

akira25014• 2 years ago

had a much larger crew capacity than most missions probably needed

We rarely flew the maximum number of passengers. On non-EVA missions we typically only sent up 5 astronauts. For EVA missions we usually sent up 7 with the two extra crew typically being dedicated to the EVA.

EVAs are a real chore. The shuttle is at 14.7 psi with regular atmosphere, but the EVA suits are 4 psi with pure oxygen atmosphere, so you have to spend a lot of time pre-breathing just to put on the suit. It also drains the hell out of you because it's microgravity, not zero gravity, and moving around and positioning in the suit using just your hands all day wears you out fast.

The extra capacity was also useful for bringing other nations personnel onto a mission with us. They didn't strictly have a large mission role, but it is good diplomacy, and helps other nations build up their own space program. Plus.. a few times.. we sent up 6 but brought back 7, which is a nice feature.

Anyways, when not sending up extra crew, we used the additional space for equipment and large experiment packages, some of them as large as a single person.

and probably used a little more than it should be.

NASA did a great job of making space flight look normal and routine. Which is a bummer, because if you dig into any of their voluminous documentation on the shuttle program as a whole, or into individual missions, they are all anything but.

Space is just an absolutely insane environment to work in, let alone to discover and study from, and the shuttle was an excellent platform for that work. Somewhere along the way space commercialization became about building stations and just trucking people back and forth to them. And for that inglorious mission the shuttle is definitely not appropriate.

Anyways.. one of my favorite things about the orbiter.. the front thermal windows need to be tinted; obviously, but what I found out recently is they used the same green dye they use in US banknotes to do that.

mihaaly• 2 years ago

Since they used aluminium in the Space Shuttle would that also reflect poor safety record on using aluminium in mission critical situations?

akira25014• 2 years ago

due to its poor safety record.

Per passenger mile traveled, the most common measure, it's one of the safest vehicles ever created and flown.

will people even remember the Space Shuttle in a good light?

This is an honest question, since my childhood was squarely in the 1980s, but how can you possibly not? Are you so young that your only perspective of this program and all it's missions and accomplishments are purely in retrospect and colored by the zeitgeist of our current civilian space contractors?

ianburrell• 2 years ago

Not even close to safest vehicle unless mean space vehicle. Space Shuttle was in orbit for 21k orbits and traveled 542 million miles. Which gives 28 deaths per billion miles.

Airliners are running 0.01 deaths per billion miles. Driving is 15 per billion miles. So it was worse than driving. Airliners beat it by 3 orders of magnitude. Which isn't surprising when US airliners travel distance of Space Shuttle in less than month.

vednig• 2 years ago

Main reason it is, as it is, and is usually not seen in any other languages is that go does not has operator functions for variable fallbacks and logical error handling. But overall this helps in improving performance of application(on a large scale).

boilerupnc• 2 years ago

Related article on Space Shuttle Software Quality [0]

Excerpt: "But how much work the software does is not what makes it remarkable. What makes it remarkable is how well the software works. This software never crashes. It never needs to be re-booted. This software is bug-free. It is perfect, as perfect as human beings have achieved. Consider these stats : the last three versions of the program — each 420,000 lines long-had just one error each. The last 11 versions of this software had a total of 17 errors. Commercial programs of equivalent complexity would have 5,000 errors."

[0] https://archive.is/HX7n4

emerongi• 2 years ago

5000 / 17 ≈ 295. Is it a fair assumption to make that a commercial program of equivalent complexity would take 295x fewer man-hours?

fleischhauf2• 2 years ago

without any knowledge of the other processes involved in space shuttle software development, I highly doubt that the correlation is that easy

burkaman• 2 years ago

No, I don't think so, 295x is a crazy high factor. The article says 260 people are involved, and let's generously say it took 20 years to write the software (the first mission was 10 years after the program started and it was around for a total of 40 years).

Dividing by 295 means a commercial team of the same size could have done it in less than a month. Or with a 10x smaller team, about 8 months. I don't think either of those are plausible.

bigiain• 2 years ago

As I skimmed that, the text wrapped at the hyphen in man-hours, and my brain autocompleted it to "295x fewer managers" - and it pretty much rings true...

fleischhauf2• 2 years ago

I wonder about spaceX track record

cpf_au• 2 years ago

One of my all-time favourite articles. Amazing that something from the internet in 1996 is still accessible!

dpedu2• 2 years ago

Consider these stats : the last three versions of the program — each 420,000 lines long-had just one error each.

What exactly do they mean by this? If each of the 3 versions had exactly one bug, isn't this just a weird way of saying the first 2 fixes either didn't work or introduced a new bug?

daemonologist• 2 years ago

Or maybe between one version and the next they only found one bug (there may have been bugs in the first version which weren't fixed until the third or later) - this seems more plausible to me since it's... rather difficult to count bugs until after you know about them.

Of course now the greatness of the feat depends on how much testing there was between versions, but given that it was the shuttle there was probably a lot.

CodeWriter23• 2 years ago

Regressions are not implied by that statement. A bug doesn’t exist in the human realm until a human observes it.

akira25014• 2 years ago

Known bug.

The SRR (software readiness review) process happened after development but prior to certification for launch. Most of the bugs were found here and were found to have existed in the code since the beginning of the program.

These were overwhelmingly low severity discrepancy reports.

If I recall correctly, there was a time when they were finding lots of bugs through SRR, so the main development team started their own "continuous review" designed to catch bugs before going to SRR. This made the SRR people angry because they were finding fewer bugs and felt the development team was focusing on competition over bug numbers rather than the code itself.

rqtwteye• 2 years ago

It would be interesting to see the NASA approach compared to how SpaceX does things. Considering that they have done manned missions they seem to have very similar requirements.

dotnet00• 2 years ago

It'd depend on what software is under consideration. IIRC the UI in Crew Dragon is using more contemporary stuff, Node.js I think. This is fine because they have redundancy, there's minimal crew control anyway, and there are manual overrides behind a panel below the screens.

They have 3 relatively modern CPUs setup to run the same code and error check each other, such that if one has an issue, there's still redundancy when it's rebooting.

The software controlling critical systems is probably closer to NASA-esque practices.

deterministic2• 2 years ago

The problem with the Space Shuttle development methology is that it is insanely expensive and slow and still not 100% bug free.

It is so expensive and slow that it would be much cheaper, faster, and actually safer to prove the software correct using modern proof assistants.

Projects like seL4, CompCert etc. shows how it should be done.

philip1209• 2 years ago

Ah, 2k-line files makes me miss my days of coding in Go.

pizzafeelsright• 2 years ago

Join me. Still crushing it.

LAC-Tech2• 2 years ago

2k lines is a whole widely used and feature filled open source library in many languages.

stitched2gethr• 2 years ago

Yes, but often density is at odds with readability.

theshrike793• 2 years ago

It's a lot easier to glance through a single 2k line file than it would be to go through 200 separate 100 line files (+ extra overhead for imports in each file)

jsbg4• 2 years ago

// 1. Every 'if' statement has a matching 'else' (exception: simple error
// checks for a client API call)
// 2. Things that may seem obvious are commented explicitly

Honest question: Why invent "safety" practices and ignore every documented software engineering best practice? 2,000 line long modules and 200-line methods with 3-4 if-levels are considered harmful. Comments that say what the code does instead of specifying why are similarly not useful and likely to go out of date with the actual code. Gratuitous use of `nil`. These are just surface-level observations without getting into coupling, SRP, etc.

bayindirh2• 2 years ago

Because sometimes, there's "No Other Way(TM)".

Arbitrary line limits tend to unnecessary fragmentation. Add includes, licenses, glue code and comment; and you have an unapproachable spaghetti.

Try to keep methods to 200 lines in high performance code, and see your performance crash and burn like Icarus' flight.

When you read the comments in the code, you can see that they simplified the code to a single module, and embedded enormous amount of know-how to keep the code approachable and more importantly, sustainable.

For someone who doesn't know the language or the logic in a piece of code, the set of comments which outline what the code does is very helpful. In six months, your code will be foreign to you, so it's useful for you, too.

Comments are part of the code and the codebase. If you're not updating them as you update the code around them, you're introducing documentation bugs into your code. Just because the compiler doesn't act on them doesn't mean they are not functional parts of your code. In essence they're your knowledge, and lab notebook embedded in your code, and it's way more valuable in maintaining the code you wrote. They are more valuable than the code which is executed by the computer.

Best practices are guidelines, not laws or strict rules. You apply them as they fit to your codebase. Do not obey them blindly and create problematic codebases.

Sometimes you have to bend the rules and make your own, and it's totally acceptable when you know what you're doing.

johnnyanmac2• 2 years ago

Try to keep methods to 200 lines in high performance code, and see your performance crash and burn like Icarus' flight.

Are these loops in Kubernetes so hot that extra microseconds for some program stack manipulation will affect performance? I never took Kubernetes as a hyper-real time application.

Do not obey them blindly and create problematic code bases.

I don't know the code so won't question it specifically, but wouldn't this also apply to "space shuttle programming"? I feel Space shuttle programming's job in many ways is in fact to try and remove ambiguity from code. But not by explaining the language, but the variables and their units. I sure wouldn't mind spamming "units in cm" everywhere or explaining every branch logic if it's mission critical. Not so much this inconsistent doxygen/javadoc style documentation on every variable/class. If you're going to go full entrprise programming, commit to it.

Above everything else, the big thing going through my mind reading these are "a proper linter configuraion would have really helped enforce these rules".

ajuc• 2 years ago

There's nothing inherently wrong with a 200-line-long method. If the code inside is linear and keeps the same level of abstraction - it can be the best option.

The alternative (let's say 40 5-line-long methods) can be worse (because you have to jump from place to place to understand everything, and you can mess up the order in which they should be called - there's 40! permutations to choose from).

slaymaker1907• 2 years ago

I tried writing in this "safe" way for quite a while, but I found the number of bugs I wrote was much higher and took way longer than just using railroad-style error handling via early returns.

The problem with having an explicit else for every if block is that the complexity of trying to remember the current context just explodes. I think a reasonable reframe of this rule would be "Every if-conditional block either returns early or it has a matching else block". The pattern of "if (cond) { do special handling }" is definitely way more dangerous than early return and makes it much harder to reason about.

mden• 2 years ago

Why invent "safety" practices and ignore every documented software engineering best practice?

That seems unnecessarily brutal (and untrue).

2,000 line long modules and 200-line methods with 3-4 if-levels are considered harmful

Sometimes, not always. Limiting file size arbitrarily is not "best practice". There are times where keeping the context in one place lowers the cognitive complexity in understanding the logic. If these functions are logically tightly related splitting them out into multiple files will likely make things worse. 2000 lines (a lot of white space and comments) isn't crazy at all for a complicated piece of business logic.

Comments that say what the code does instead of specifying why are similarly not useful and likely to go out of date with the actual code.

I don't think this is a clear cut best practice either. A comment that explains that you set var a to parameter b is useless, but it can have utility if the "what" adds more context, which seems to be the case in this file from skimming it. There's code and there's business logic and comments can act as translation between the two without necessarily being the why.

Gratuitous use of `nil`

Welcome to golang. `nil` for error values is standard.

ljm• 2 years ago

There is no single canonical suite of best practices.

There is also nothing harmful or unharmful about the length of a function or the lines of code in a file. Different languages have their opinions on how you should organise your code but none of them can claim to be ‘best practice’.

Go as a language doesn’t favour code split across many small files.

johnnyanmac2• 2 years ago

It's all opinions and "best practice" isn't some objective single rule to uphold.

But generally, best practices are "best" for a reason, some emperical. The machine usually won't care but the humans do. e.g. VS or Jetbrains will simply reject autocompletion if you make a file too big, and if you override the configuration it will slow down your entire IDE. So there is a "hard" soft-limit on how many lines you put in a file.

Same with Line width. Sure, word wrap exists but you do sacrifice ease and speed of readability if you have overly long stretches of code on one line, adding a 2nd dimension to scroll.

jsbg4• 2 years ago

There is no single canonical suite of best practices.

There kind of is, though. Most software engineering books argue for the same things, from the Mythical Man Month to Clean Architecture.

Different languages have their opinions on how you should organise your code

In general best practices are discussed in a language-agnostic manner.

cellularmitosis• 2 years ago

If you think these things are considered harmful, I'd encourage you to read "John Carmack on Inlined Code" http://number-none.com/blow/john_carmack_on_inlined_code.htm...

"The flight control code for the Armadillo rockets is only a few thousand lines of code, so I took the main tic function and started inlining all the subroutines. While I can't say that I found a hidden bug that could have caused a crash (literally...), I did find several variables that were set multiple times, a couple control flow things that looked a bit dodgy, and the final code got smaller and cleaner."

If Carmack finds value in the approach, perhaps we shouldn't dismiss it out of hand.

Also worth noting his follow-up comment:

"In the years since I wrote this, I have gotten much more bullish about pure functional programming, even in C/C++ where reasonable... When it gets to be too much to take, figure out how to factor blocks out into pure functions"

jsbg4• 2 years ago

Thanks. This is the first instance of a respected software engineer arguing in favor of this style that I have read (contrast with Dave Thomas, Kent Beck, Bob Martin, etc.)!

anonymoushn• 2 years ago

Is there any evidence that these things are harmful or just vibes?

high_na_euv3• 2 years ago

200-line methods with 3-4 if-levels are considered harmful.

Maybe if you are in love with software evangelists (bullshitters) like Uncle Bob

jsbg4• 2 years ago

I would like to hear about what makes them bullshitters. I've had and seen really good results in terms of high productivity and low bug count on teams that followed the SOLID principles described in Robert Martin's Clean Architecture as well as Kent Beck's "make it work, make it right, make it fast." I've also universally observed the opposite results on teams that didn't.

gilbetron• 2 years ago

Splitting a 200 line method into 20, 10-line methods rarely improves readability, it just tricks you into thinking those 200 lines are simpler than they actually are.

Furthermore, how to split 200 lines into methods is context dependent. Looking through the lens of memory, optimality, simplicity, different flows of concern, and you'll want to split those 200 lines up differently.

The problem space is complex, hiding that fact doesn't get rid of that fact.

ChrisArchitect• 2 years ago

Discussion from 2018: https://news.ycombinator.com/item?id=18772873

whalesalad11• 2 years ago

wtf I submitted this in 2018?????

pranshum• 2 years ago

Our obsessions never change!

daedrdev• 2 years ago

The joys of the human memory

scj• 2 years ago

That begs the question: What brought you to this file?

dannyobrien• 2 years ago

good thing thing you wrote a comment here so you can remember what you were thinking next time you read it :)

aitchnyu• 2 years ago

I sometimes read posts/comments on pages I got from Google and then realize I made those comments.

EgoIsMyFriend• 2 years ago

Previous discussion https://news.ycombinator.com/item?id=18772873 (Dec 2018, 1552 upvote, 631 comments)

dangsux• 2 years ago

By the same user no less

renewiltord2• 2 years ago

Why was Space Shuttle code so good and the engineering so bad? The thing was expensive and shit and had a 1.5% catastrophic failure rate for passenger transport. Soyuz was two orders of magnitude better.

Russia/USSR have reputation for McGyvering things and US has reputation for gold-plating but US ship is killing people every 65 flights and Russian ship has over 1500 launches without death.

Maybe engineers should learn from either Soviet/Russian engineers or from NASA software engineers. Both are making more reliable things.

teractiveodular• 2 years ago

Multiple books have been written on this very topic, but the TL;DR is that the problem was not the engineering, but the absurd, often mutually contradictory design decisions forced on it for political reasons.

https://en.wikipedia.org/wiki/Space_Shuttle_design_process

nvarsj2• 2 years ago

The irony is this kind of describes k8s as well.

ranger207• 2 years ago

https://waynehale.wordpress.com/2012/11/12/after-ten-years-a...

I need to give you the issue from the NASA point of view so you can understand the pressures that they were under. In a developmental program, any developmental program, the program manager essentially has four areas to trade. The first one is money. Obviously, he can go get more money if he falls behind schedule. If he runs into technical difficulties or something goes wrong, he can go ask for more money. The second one is quantity. The third one is performance margin. If you are in trouble with your program, and it isn’t working, you shave the performance. You shave the safety margin. You shave the margins. The fourth one is time. If you are out of money, and you’re running into technical problems, or you need more time to solve a margin problem, you spread the program out, take more time. These are the four things that a program manager has. If you are a program manager for the shuttle, the option of quantity is eliminated. There are only four shuttles. You’re not going to buy any more. What you got is what you got. If money is being held constant, which it is—they’re on a fixed budget, and I’ll get into that later—then if you run into some kind of problem with your program, you can only trade time and margin. If somebody is making you stick to a rigid time schedule, then you’ve only got one thing left, and that’s margin. By margin, I mean either redundancy—making something 1.5 times stronger than it needs to be instead of 1.7 times stronger than it needs to be—or testing it twice instead of five times. That’s what I mean by margin.
It has always been amazing to me how many members of Congress, officials in the Department of Defense, and program managers in our services forget this little rubric. Any one of them will enforce for one reason or another rigid standard against one or two of those parameters. They’ll either give somebody a fixed budget, or they’ll give somebody a fixed time, and they forget that when they do that, it’s like pushing on a balloon. You push in one place, and it pushes out the other place, and it’s amazing how many smart people forget that.

twobitshifter• 2 years ago

Soyuz doesn’t look like it came from a movie and can’t fly like a plane to touch down on a runway.

Merad• 2 years ago

Where are you getting 1500 Soyuz flights? As far as I can find the number is more like 150. It also experienced failure and loss of crew on two missions (Soyuz 1, Soyuz 11).

renewiltord2• 2 years ago

It's correct. I have confused Soyuz launcher with Soyuz spacecraft like newbie. It's true. Soyuz spacecraft has early failures (till 1971, none after). Shuttle has late failures (2003, then project decommission). I suppose design improvement in Soyuz, engineering quality decrease in Shuttle. Incompetent early Soyuz design team. Incompetent late Shuttle engineering team. Lack of ethics to make such broken devices.

neilv• 2 years ago

BTW, when linking to a file in GitHub like this, you can link to a range of lines of the file, by using the URL fragment identifier, like:

    #L60-L92

https://github.com/kubernetes/kubernetes/blob/60c4c2b2521fb4...

runlevel1• 2 years ago

You can click a line number to select the start of the range, then shift+click to select the end of the range to do this automatically.

ivanjermakov• 2 years ago

I think the whole file worth skimming through

whalesalad11• 2 years ago

Try submitting a URL to HN that has an anchor tag and see what happens =)

corytheboyd• 2 years ago

I (well really, a coworker of mine) just today discovered a JetBrains action “Copy GitHub URL” [sic] that, if you have lines of code selected in the IDE, includes those lines in the copied URL fragment. So so so much better than my old workflow of stopping what I’m doing, going to the file in GitHub, and selecting the lines there to share links to bits of code.

croemer• 2 years ago

Same exists in VScode, probably need some GitHub extension for it.

kylehotchkiss• 2 years ago

Is this like a thing where people want to appear to be contributing to open source and remove all the 'unneeded' elses from a codebase?

quotemstr• 2 years ago

This sort of code strikes me as an ideal candidate for translation into a declarative, rule-based, table-driven system. Such a thing is more comprehensible and more verifiable than ad-hoc imperative if-clause-rich code.

Messy code of this sort is usually a sign of a missing abstraction.

umanwizard3• 2 years ago

The Go ideology is basically to just write down all the code, in a more or less straightforward translation of what you would have written in C, and not to try to abstract anything.

GuB-42• 2 years ago

Which is an underrated ideology.

I once had to go through some code that was objectively terrible. The guy who wrote was is a mechanical engineer, close to retirement at the time, and self-learned in programming. He had absolutely none of the background you can expect a professional programmer to have, and in particular abstraction seemed like a foreign concept to him. Have 50 buttons, each doing essentially the same thing, and you will see 50 copy-pasted blocks of code. Particularly ironic that he was using Java, a language known for its culture of abuse of design patterns and abstraction.

But despite that huge mess that code was, it was surprisingly readable. You could look anywhere in the code and understand what it is doing. No calling though interfaces, when you see foo.bar(), you can just follow the symbol in your IDE and that's the instruction that will be run, and many times, there is not even a function, just code, thousands of lines of it, different cases are just dealt with ifs.

Maybe it was the most pleasant "bad code" I had to work with. Code using the wrong or too much abstraction is much worse, because it is just as buggy and ugly, but you don't even know what to expect when all you see is an interface call where the actual code is in a completely different part of the software, and what connect the two is in yet another place. With more "factories", "managers", "dispatchers", etc... than actual logic.

copypasterepeat• 2 years ago

I've obviously only skimmed the code, but honestly it doesn't look that bad to me. Sure there are things I would do differently, but I've seen much, much worse. At least the code follows a single convention, and has the appearance that everything was thought through and that there is method behind the madness, as it were. I'd take this any day over the typical mishmash of styles, lazy coding, illogical code structure etc. that I've encountered so many times.

matthewmacleod• 2 years ago

Personally I love this level of verbosity in code. There are still way too many levels of nested control flow for my taste—I find that makes it exceptionally hard to retain context—but at least there are early returns.

theshrike793• 2 years ago

With a half decent editor you can fold them down so that just the comment on top of the if branch is visible

dpedu2• 2 years ago

I wrote a toy kubernetes CSI driver recently and found it quite pleasing to do. Bare minimum means implementing just 3 grpc api calls - An informational one and publish/unpublish. Amazon's EFS or EBS CSI drivers are good examples because they're a pretty small codebase. I don't know exactly how this code interacts with the CSI driver itself, but it appears that it is the logic that ultimately results in the volume manipulation calls the controller makes against the CSI driver. It's nice that the complexity is all here, I was actually pretty surprised how simple the drivers themselves are.

wavemode2• 2 years ago

Am I weird in feeling like the code in this file is really really... normal? Like, it's verbose in certain ways due to being written in Go, as well as due to not relying on any deep abstractions (and I don't mind this - abstractions are a double-edged sword), but in general, as code, it seems typical - and if the header text didn't exist I wouldn't think twice about the style it's written in.

Maybe the disconnect here is that most of my experience is in enterprise software rather than systems software. Perhaps many of the comments in this file seem unnecessary to regular contributors within the k8s project? Whereas if I were writing this same code in an enterprise (and thus expect it to be read by people far in the future lacking context on all the technical details) I would have put -more- comments in this file, given the sheer complexity of all it's doing.

mumblemumble3• 2 years ago

I wish code like this still felt normal to me, but over the past ~10 years it seems that many people have come to value brevity over explicitness.

I strongly prefer the explicitness, at least for important code like this. More than once in my career I've encountered situations where I couldn't figure out if the current behavior of a piece of code was intentional or accidental because it involved logic that did things like consolidating different conditions and omitting comments explaining their business context and meaning.

That's a somewhat dangerous practice IMO because it creates code that's resistant to change. Or at least resistant to being changed by anyone who isn't its author, though for most practical purposes that's a distinction without a difference. Unnecessarily creating Chesterton's fences is anti-maintainability.

bhauer2• 2 years ago

Agreed. Explicitness and comments are very useful in understanding the intended functionality and logic, whether or not the code actually implements that intent correctly (an in providing that intent, they can help identify bugs earlier than they would be identified otherwise).

spacemadness• 2 years ago

Every time some code reviewer comes into my PR and says something along the lines of "you know you can just write it this way" where "this way" means obfuscating the code because "clever" and "shorter," I die a little on the inside. This is from experienced devs who should know better. At one point I wrote a comment write above a section I knew would be targeted by this kind of thinking explaining it must be written this way for clarity. Sometimes that works.

DSMan1952762• 2 years ago

I agree to a point, but I would separate explicit code from excessive commenting. Explicit code is good because it lets you explain to the reader what you're actually trying to do. Excessive comments (or even comments in general) is less so because compiler cannot check them for correctness, if someone simply forgets to update a comment or writes it incorrectly then the only thing to potentially catch it is a code review.

claytongulick4• 2 years ago

I strongly prefer the explicitness

I have a rule for my teams: "Don't write clever code".

I try to constantly reinforce that we don't write code for ourselves, we write it for the next person.

We should be doing everything in our power to decrease their cognitive load.

I try to envision the person that comes after me (who may be me in months or years!) and imagine that they are having a Bad Day and they have to make changes to my code.

Good code is clear, and tells a story. A story that's easy to follow, and easy to drill into.

Not to knock elixir unfairly, but I think that's the basis of my mental block with that language. It seems to be designed from the ground up to violate that rule. Everything is elixir is clever. Very clever. Too clever for me.

Buttons840• 2 years ago

The first developer I ever worked with was very explicit, and I learned some important lessons from him. He had created a system in PHP that controlled printers and it was very explicit. He didn't know what a function was, his code had no functions. It was a 5000 line script that would run on a Windows timer, top to bottom. In some places the control structures were nested 17 deep, I counted; an if-statement inside an if-statement inside a while-loop inside a for-loop inside an if-statement inside a for-loop inside an if-statement, etc, 17 deep. The cyclomatic complexity was in the thousands.

I never could understand that code. I know it wasn't brief, does that mean it was explicit?

I think the truth is brevity and explicitness are orthogonal. Let me ask you this: can code be both brief and explicit? What would that look like?

motherfsck• 2 years ago

I used to have opinions but I don't care anymore. After a spelunking into all manner of code bases I just match the style and move on. Verbose or terse, comments at the top or inline or nor none at all, 100 or 5 line functions, etc.

I treat it like a card game; follow what's led.

bhauer2• 2 years ago

You may be weird, but you're not alone. I too think this looks perfectly normal. I have written similar-looking code and comments (at least from a high-level point of view) for any components that I feel are critical to the reliability of the system. I've never subscribed to the "no comments" fad since my own comments have all too often been invaluable to future me when returning to the code after months or years of attention elsewhere. I can't imagine trying to piece together all of the embedded logic in a component of this complexity without solid comments.

remram• 2 years ago

I know. I would never remove an "empty" branch full if comments, that's the compiler's job.

zdragnar• 2 years ago

The one thing that stood out to me is the rather extensive nesting of if-else.

I know this is go code, but there's multiple places that go five branches deep, at least.

It works but it's a style that takes a lot of effort to grow what's happening IMHO.

omoikane• 2 years ago

Most likely, this comment was added in response to a botched attempt to simplify code, to serve as a warning for future maintainers to think twice before making a similar attempt.

The commit that added the warning was "Add note about space-shuttle code style"[1], and the one before that was "Revert controller/volume: simplify sync logic in syncUnboundClaim"[2]

[1] https://github.com/kubernetes/kubernetes/commit/de4d193d45f6...

[2] https://github.com/kubernetes/kubernetes/commit/8a1baa4d64ca...

croshan• 2 years ago

Nice find. I think you're right.

FWIW, I don't think the code style in [2] is less simple (slightly more readable, to use `if (X) {} else {}` rather than `if (!X) {} else {}`, for example).

So to me, this reads as the author of [1] is just overcorrecting by adding process, when some test cases or code review would've been more helpful in preventing whatever incident [2] caused.

hyperpape• 2 years ago

In particular, the every if has a matching else comment doesn't seem reliably true. Many of the unmatched ifs are just simple if (err != nil) { checks, but or other early returns, but outside of those, there do seem to be unmatched ifs.

That said, my experience in enterprise software isn't that extra comments are necessarily present (there was a plague of "// end if" comments in the codebase, but actual descriptive comments were rare).

wavemode2• 2 years ago

Yeah I should clarify that I was speaking personally about new code. I definitely encounter plenty of legacy code that is completely inscrutable.

But, I can say in 2024 at least, that most teams I've been on value explaining complex logic (especially logic that can break in subtle ways if not properly maintained) with comments, in new code we write.

marcus_holmes4• 2 years ago

I found myself thinking the same thing until I got to the hugely nested if statements. I would definitely have created some early-return branches to that thing.

It does feel like the first "make it work" step (from "make it work, make it fast, make it pretty"), and then they just didn't do the "make it pretty" step. I have written code this "ugly" before, with this many comments, when working out a thorny state interaction. But I usually clean it up a bit before submitting for review. Maybe instead I should just put a huge "do not attempt to simplify this code" banner at the top hehe :)

But yes, it's not that bad, for sure.

al_borland• 2 years ago

In most code I look at, to try and get clarity on something I’m trying to do, I have no idea what’s going on. Variables seem arbitrary, everything seems cryptic, and I can’t be bothered to try and track down what’s happening.

My own code doesn’t look like that at all. I have long descriptive variables, what I think are easy to read and follow functions, etc. Sometimes I think if I want to be “good” I need my stuff to look like what I find, but at the end of the day, I want it to be easy for me (and hopefully others) to maintain. If no one else can read my code, I don’t view it as job security, I view it has handcuffs.

zzo38computer• 2 years ago

I looked through some parts of it, and it look like to me that there is many redundant stuff in many if/else blocks.

BaculumMeumEst• 2 years ago

I do like the thought of firing Kubernetes into space.

sam_perez• 2 years ago

Beautiful.

----------------------------

This controller is intentionally written in a very verbose style. You will notice:

1. Every 'if' statement has a matching 'else' (exception: simple error checks for a client API call)

2. Things that may seem obvious are commented explicitly We call this style 'space shuttle style'. Space shuttle style is meant to ensure that every branch and condition is considered and accounted for - the same way code is written at NASA for applications like the space shuttle.

----------------------------

^^^^

This bit reminds me of exhaustive checks in typescript code. I try to use them all the time.

https://www.typescriptlang.org/docs/handbook/2/narrowing.htm...

shepherdjerred• 2 years ago

Please do not attempt to simplify this code

Related Stories